387.-First-Unique-Character-in-a-String#14
Conversation
There was a problem hiding this comment.
Nit ですが、md 配下に code block 書くと syntax highlight ちゃんと機能していない箇所がありますね。VS Code ではちゃんと機能しているのでGitHub の仕様っぽいですが。
There was a problem hiding this comment.
md中に空行があると変になってる気がしますね。
自分は見にくいときはrich diffから見ています。
| // Return the index of the first character which appears exactly once in the string, | ||
| // or -1 if no such character exists. | ||
| int firstUniqChar(const std::string& s) { | ||
| std::map<char, int> char_to_first_appear_index; |
| int firstUniqChar(const std::string& s) { | ||
| std::set<char> repeated_chars; | ||
| std::map<char, int> candidate_char_to_index; | ||
| for (int i = 0; i < s.size(); ++i) { |
There was a problem hiding this comment.
C++ のお作法よくわからないのですが、ここについては代入後の式の値を使うわけではないので ++i でも i++ でも挙動に違いがないんですね。
https://qiita.com/suuungwoo/items/e054fdcb5a4805bb226b
https://qiita.com/baby-degu/items/901f22c7758d3f619a33
There was a problem hiding this comment.
この場面では前置でも後置でもあまり違いはないと思います。後置インクリメントit++については、インクリメント前の値が必要でコピーが生成されるため、必要がない限り使わない使わないほうがよいようです。そのため紛らわしいので基本前置インクリメントを使うようにしています。
irohafternoon/LeetCode#22 (comment)
| auto c = s[i]; | ||
| if (repeated_chars.contains(c)) { | ||
| continue; | ||
| } else if (candidate_char_to_index.contains(c)) { |
There was a problem hiding this comment.
ifで十分なことを見落としていました。ありがとうございます。
| public: | ||
| int firstUniqChar(const std::string& s) { | ||
| std::set<char> repeated_chars; | ||
| std::map<char, int> candidate_char_to_index; |
There was a problem hiding this comment.
unique_char_to_index の方が実態を表しているような気がします。
There was a problem hiding this comment.
なるほど。後で消しうるというのでcandidateとしたのですが、それも確かにそうですね…
| #include <string> | ||
|
|
||
| class Solution { | ||
| public: |
There was a problem hiding this comment.
空白を 3 個入れ停電とするのはあまり見かけないように思います。 public: であればインデント無しか空白 2 個が多いように思います。通常の行のインデントが 2 個の場合は、 1 個にする場合もあるようです。
参考までにスタイルガイドへのリンクを貼ります。
https://google.github.io/styleguide/cppguide.html#Horizontal_Whitespace
// Spaces around the colon in inheritance and initializer lists.
class Foo : public Bar {
public:上記のスタイルガイドは唯一絶対のルールではなく、複数あるスタイルガイドの一つに過ぎないということを念頭に置くことをお勧めします。また、所属するチームにより何が良いとされているかは変わります。自分の中で良い書き方の基準を持ちつつ、チームの平均的な書き方で書くことをお勧めいたします。
There was a problem hiding this comment.
ありがとうございます。違和感がありつつもフォーマッタの動作に任せてしまっていたのですがやはり一般的ではないのですね。よく見られる書き方に合わせようと思います。
| std::map<char, int> char_to_first_appear_index; | ||
| for (int i = 0; i < s.size(); ++i) { | ||
| if (char_to_first_appear_index.contains(s[i])) { | ||
| char_to_first_appear_index[s[i]] = -1; |
There was a problem hiding this comment.
-1 のような特別な値を使うと、バグを購入しやすくなる印象があります。定数においてあげれば、やや緩和できるように感じます。
There was a problem hiding this comment.
非負整数でないことを表現したかったのですが確かにマジックナンバーになってしまってますね。このような数字を使わざるを得ないときはせめて定数化するようにします。
| https://github.com/quinn-sasha/leetcode/pull/15/files?short_path=5ec7c3c#diff-5ec7c3c87171edf4d61e9eb79fd926cafa27caf068da7474222897c8e9e7ab96 | ||
| ```cpp:step2-3.cpp | ||
| #include <algorithm> | ||
| #include <map> |
There was a problem hiding this comment.
mapではなくunordered_mapを使うことを検討しても良いかもしれません。
(C++素人なので的外れだったらご放念ください。)
参考:https://note.com/modern_ferret431/n/n48b154df7362
4 participants
This Problem
https://leetcode.com/problems/first-unique-character-in-a-string/