Conversation
Added detailed explanation and Java implementation for finding the first unique character in a string.
There was a problem hiding this comment.
[IMO] コードとは関係ないのですが、ディレクトリ名やファイル名に (スペース)を入れるのは好みではありません。CLIなどでエスケープが必要だったりと実務上の手間が増えるからです。
私はKth_Largest_Element_in_a_Stream/memo.mdみたいな感じで _ を入れてます。
There was a problem hiding this comment.
返信が遅くなり、大変失礼いたしました。
ファイル名の作法があまりわかっていませんでしたが
いろいろとデメリットがありそうですね。
次からは、スペースは使わず_を使用しようと思います。
レビューいただきありがとうございます。
TakayaShirai
left a comment
There was a problem hiding this comment.
Step 2 で、想定されていない入力が渡されるケースなど、さまざまなケースについて考慮されていたのが非常に良いと思いました!
| Set<Character> repeating = new HashSet<>(); | ||
| int n = s.length(); | ||
| for (int i = 0; i < n; i++) { | ||
| char c = s.charAt(i); |
There was a problem hiding this comment.
最初に読んだ際に repeating が何を表しているのかが不明瞭であるように感じました。おそらく repeating と duplicated が、どちらも「重複」という同じ意味だが違う表現になっていることが原因だと思います。
命名としては seen などをよく見るので、repeating の代わりにこちらもご検討ください。
また、duplicated という命名の Set を持ってしまって、if (!duplicated) の部分をフラグではなく、if (!duplicated.contains(c))として、Set で条件分岐を行うのもありかなと思いました。
以下参考まで。
class Solution {
public int firstUniqChar(String s) {
Set<Character> duplicated = new HashSet<>();
int n = s.length();
for (int i = 0; i < n; i++) {
char c = s.charAt(i);
if (duplicated.contains(c)) {
continue;
}
for (int j = i + 1; j < n; j++) {
if (c == s.charAt(j)) {
duplicated.add(c);
break;
}
}
if (!duplicated.contains(c)) {
return i;
}
}
return -1;
}
}There was a problem hiding this comment.
返信が遅くなり、大変失礼いたしました。
確かに、duplicatedとrepeatingは同じような意味なので、コードの意味が取りにくいかもしれません。
フラグを使ってみたくて、duplicatedフラグをやってみたのですが、なくても問題ないですね。
似たような変数名が存在してないかや別の表現でできないか、気を配ろうと思います。
レビューいただきありがとうございます。
| @@ -0,0 +1,178 @@ | |||
| //STEP1 | |||
There was a problem hiding this comment.
本筋とは関係ないですが、Markdownの記法に従っておくとレンダリングしたときに見やすいです。
| //STEP1 | |
| ## STEP1 |
There was a problem hiding this comment.
返信が遅くなり、大変失礼いたしました。
mdファイルがマークダウンという記法で書かれたファイルであること、このコメントで理解しました。
記法を調べて、それにのっとって書くようにします。
レビューいただきありがとうございます。
| @@ -0,0 +1,178 @@ | |||
| //STEP1 | |||
|
|
|||
| ■手作業でどうやるか? | |||
| 上記のC++の解答ではunordered_mapを使用しているが、javaではmapは最初からunorderedということなので、今回は気にしなくてよさそう。 | ||
|
|
||
| https://stackoverflow.com/questions/7792523/hashmap-gives-an-unordered-list-of-values |
There was a problem hiding this comment.
何を気にしているのか次第ですが、私の場合、この箇所を読んだときに unordered ということは「orderedであることが期待されうるのに、そうでないこと」を明示しているのだな、と感じました。
Javaに詳しくないので調べてみました。
https://developer.android.com/reference/java/util/HashMap
This class makes no guarantees as to the order of the map; in particular, it does not guarantee that the order will remain constant over time.
https://developer.android.com/reference/java/util/LinkedHashMap
This linked list defines the encounter order (the order of iteration), which is normally the order in which keys were inserted into the map (insertion-order). The least recently inserted entry (the eldest) is first, and the youngest entry is last.
HashMap は挿入順に取り出せることを保証しませんが、LinkedHashMap は保証しています。
ちなみに、keyをソートしてくれる TreeMap というのもあるみたいですね。
https://developer.android.com/reference/java/util/TreeMap
Note that the ordering maintained by a tree map, like any sorted map, and whether or not an explicit comparator is provided, must be consistent with equals if this sorted map is to correctly implement the Map interface.
There was a problem hiding this comment.
Java は TreeMap と HashMap があって、C++ の map と unordered_map に大まかに対応しています。
この2つの中身がどんな形かは大まかに知っておいていいでしょう。
There was a problem hiding this comment.
mamo3grさん、odaさん
Mapの種類についてコメントやリンクのご提示などありがとうございます。
問題を解くことだけを考えていましたが、
どんな種類があるかを把握したうえで、
適切に選択することができるよう頭に入れたいと思います。
arai60の2週目をやる時になったら、javaではない言語でもやってみたいと思います。
| for (int i = 0; i < s.length(); i++) { | ||
| if (counter.get(s.charAt(i)) == 1) { | ||
| return i; | ||
| } | ||
| } |
There was a problem hiding this comment.
counter が挿入順に要素 (key-value) を取り出せるなら、s を参照せずに、はじめに1が出てきた時点で返せそうですね。
There was a problem hiding this comment.
LinkedHashMap というものはあります。
https://docs.oracle.com/javase/jp/8/docs/api/java/util/LinkedHashMap.html
There was a problem hiding this comment.
LinkedHashMap は、個人的には知らなくてもいいんですが、しかし、面接ではよく聞かれます。
| class Solution { | ||
| public int firstUniqChar(String s) { | ||
| Set<Character> repeating = new HashSet<>(); | ||
| int n = s.length(); |
There was a problem hiding this comment.
nは定義せずに済ませても良いのではないかと思いました(好みの範囲かもしれません)
There was a problem hiding this comment.
返信遅くなり、大変失礼いたしました。
ここは、あまり深く考えずに書いていたのですが、
確かにnの定義はせず、
s.length()そのままでも良さそうですね。
変数定義の必要性を考えた上で書くことに
気をつけたいと思います。
レビューいただきありがとうございます。
今回の問題
387. First Unique Character in a String
次の問題
929. Unique Email Addresses
明けましておめでとうございます。
プライベートでばたばたしており、前回の349のプルリクエストからだいぶ時間が空いてしまいました。
また問題を解き進めていこうと思っておりますので、
どうぞよろしくお願いいたします。