Exercise 2: Reviewing a review #1
Conversation
| @@ -0,0 +1,18 @@ | |||
| import java.util.Scanner; | |||
|
|
|||
| public final class ConsoleView implements View { | |||
There was a problem hiding this comment.
There should be at least some basic Javadoc on public all classes/methods
| private final Scanner scanner = new Scanner(System.in); | ||
|
|
||
| public void show(String text) { | ||
| // Prints `text` on the console |
There was a problem hiding this comment.
Not very useful?
| @@ -0,0 +1,32 @@ | |||
| import java.io.*; | |||
There was a problem hiding this comment.
IMHO wildcard imports are bad
|
|
||
| public List<String> names() { | ||
| var result = new ArrayList<String>(); | ||
| for (var entry : root.entrySet()) { |
There was a problem hiding this comment.
Use .stream() instead for consistency with the method below?
| public Map<String, String> translatedNames(String language) { | ||
| return root.entrySet().stream() | ||
| .map(e -> e.getValue().getAsJsonObject()) | ||
| .filter(p -> p.has(language)) |
There was a problem hiding this comment.
Why do some entries not have a translation? Should we document this?
|
|
||
| var parts = command.split(" "); | ||
| if (parts[0].equals("searcch")) { | ||
| showList(model.names().stream().filter(n -> n.contains(parts[0])).toList()); |
There was a problem hiding this comment.
| showList(model.names().stream().filter(n -> n.contains(parts[0])).toList()); | |
| showList(model.names().stream().filter(n -> n.contains(parts[1])).toList()); |
| } | ||
|
|
||
| public void onInput(String command) { | ||
| if (command.equals("list")) { |
There was a problem hiding this comment.
This should support more input languages than English
| void namesAreLoaded() { | ||
| var model = new LocalDatabaseModel(); | ||
| var names = model.names(); | ||
| assertThat(names, hasItems("Sextans Dwarf Spheroidal Galaxy")); |
There was a problem hiding this comment.
Test the order of names?
|
|
||
| private static final class FakeView implements View { | ||
| public List<String> inputs = new ArrayList<>(); | ||
| public List<String> outputs = new ArrayList<>(); |
There was a problem hiding this comment.
Public fields are bad
| } | ||
|
|
||
| private void showList(List<String> list) { | ||
| view.show(list.stream().map(s -> "- " + s).collect(Collectors.joining("\n"))); |
There was a problem hiding this comment.
| view.show(list.stream().map(s -> "- " + s).collect(Collectors.joining("\n"))); | |
| view.show(list.stream().map(s -> "* " + s).collect(Collectors.joining("\n"))); |
I like bullet points more
Which comments are useful? Which are not? Should there be more?
Open the "Files changed" tab to start