387. First Unique Character in a String#20
Conversation
| class Solution { | ||
| // leetcode | ||
| public int firstUniqChar(String s) { | ||
| Map<Character, Integer> indices = new HashMap<>(); |
There was a problem hiding this comment.
変数名 indices から、文字と出現頻度のマップだということが分かりにくく感じました。 charcterToCounts や characterToFrequencies といった、文字と出現頻度のマップであることが想起しやすい変数名を付けることをお勧めいたします。
| class Solution { | ||
| // leetcode | ||
| public int firstUniqChar(String s) { | ||
| Map<Character, Integer> indices = new HashMap<>(); |
| class Solution { | ||
| // leetcode | ||
| public int firstUniqChar(String s) { | ||
| int[] cs = new int[26]; |
There was a problem hiding this comment.
cs という変数名からは、中にどのような値が入っているかを推測するのが難しく感じました。 step 2-1 のように推測しやすい英単語、または英語句を使用するとよいと思います。また、配列やコレクションのように、複数の値が含まれる変数については、最後の単語を複数形にすると、理解しやすくなると思います。
| @@ -0,0 +1,21 @@ | |||
| class Solution { | |||
| // leetcode | |||
| private static final int ALPHABET_SIZE = 26; | |||
There was a problem hiding this comment.
自分なら NUM_ALPHABETS と名付けると思います。ここは好みかもしれません。
There was a problem hiding this comment.
小文字のアルファベットの数のことなので、大きさを表す size は適当でないように思います。
There was a problem hiding this comment.
このままでも良さそうです。https://csrc.nist.gov/glossary/term/alphabet_size
There was a problem hiding this comment.
ちょっと上記のソース読んでみたのですが、どうやら上記の alphabet とは一般的に想起される abc... とは違うみたいです。全く専門外なのであまり自信がないですが、ソースになっている論文での使われ方を見るに size は可変であるようなので、恐らく形式言語における alphabet の事を指しているのかと思われます。
abc... の方の alphabet について、これの size が a-z の数26を指すという例は、私の方で少しググった限りでは見つけられませんでした。
ただ wikipedia に "An alphabet may have any cardinality ("size") and, depending on its purpose, may be finite (e.g., the alphabet of letters "a" through "z")" とあるように、形式言語における alphabet は abc... の方の alphabet も指し得るみたいなので、一般的かはさておき間違いではないですね。
There was a problem hiding this comment.
(もし英語ネイティブで形式言語についての知識がある方がいらっしゃったら意見伺いたい...)
There was a problem hiding this comment.
https://www.ldoceonline.com/dictionary/alphabet
a set of letters, arranged in a particular order, and used in writing
setに対してsizeを使うのは、普通の表現だと思います。
例：https://zoo.cs.yale.edu/classes/cs467/2005f/course/handouts/ho15.pdf
There was a problem hiding this comment.
setに対してsizeを使うのは、普通の表現だと思います。
なるほど確かに。
| return i; | ||
| } | ||
| } | ||
| return NOT_FOUND; |
There was a problem hiding this comment.
LeetCodeの制約上仕方がないですが、そもそも-1を返すことが良いことではないので、これが本当に良いのか（根本的な解決になっていないので）については個人的には疑問があります。それよりもLeetCodeの制約がないのであれば、どのように書くかということについて論じる（または実際に書いてみる）ほうが良いのではないかと思います。
There was a problem hiding this comment.
おっしゃるとおり実際のシステムだったら
・-1を返して呼び出し側でエラーチェックする
・この関数でエラーを返す
・そもそも入力値のバリデーション関数を別で設ける
といったことが候補に上がるかなと思っています。（結構既存のシステムのお作法（通例）にも影響される部分もあるイメージ）
There was a problem hiding this comment.
あ、はい、ここは「使う人の気持ちになって私はこうするのが一番いいと思ったのでこうしたのだ、もしかしたらもっといい方法があるのかもしれないが、少なくとも私はそう判断したのだ」と言えるならばいいと思います。
小学校の低学年の頃に、母に「きれいな字を書けるかは能力であるから人による。しかし、誠実な字を書くことは誰にでもできる。誠実な字とは読み手に伝えようという意思を持った字である。」といわれたことを思い出します。
| public static final int NOT_FOUND = -1; | ||
|
|
||
| public int firstUniqChar(String s) { | ||
| int[] frequency = new int[ALPHABET_SIZE]; |
There was a problem hiding this comment.
頻度の配列であることがわかるようにfrequenciesと複数形にすべきではないかと、自分は以前にnodaさんからコメントをもらいました
6 participants
問題：https://leetcode.com/problems/first-unique-character-in-a-string/