Review Task 1 #1
Conversation
| } | ||
|
|
||
| /** | ||
| * Adds a text comment. |
| } | ||
| } | ||
|
|
||
| // public void addVideoComment(File videoFile) throws CommentNotAddedException { |
| return this.comments; | ||
| } | ||
|
|
||
| public boolean hasConversation() { |
There was a problem hiding this comment.
Metoda nie tylko zwraca warunek, ale robi też coś pobocznego - "markConversation"
| addLineContent(syntaxColor); | ||
| } | ||
|
|
||
| public int get() { |
There was a problem hiding this comment.
Metoda nie mówi, co właściwie pobieramy
| * @param recordedFile | ||
| * @throws CommentNotAddedException | ||
| */ | ||
| public void createVoiceComment(File recodedFile) throws CommentNotAddedException { |
There was a problem hiding this comment.
Inconsistency of naming - tutaj mamy create, wcześniej add.
| Comment voiceComment = new Comment(AbstractComment.Type.VOICE, this); | ||
| voiceComment.setFile(recodedFile); | ||
| comments.add(voiceComment); | ||
| if (comments.size() > 0) { |
There was a problem hiding this comment.
Duplikacja kodu - może warto wyłuskać nową metodę?
| @SuppressLint("ViewConstructor") | ||
| public class Line extends LinearLayout implements Serializable { | ||
| private static final long serialVersionUID = 3076583280108678995L; | ||
| private static final int TWO = 2; |
There was a problem hiding this comment.
Brak spójności nazewnictwa zmiennych prywatnych
| // } | ||
|
|
||
| private void addLineContent(boolean syntaxColor){ | ||
| if (!syntaxColor || !SyntaxHighlighter.canBeHighlighted(syntaxColor)) |
There was a problem hiding this comment.
Warunek w ifie można uczynić bardziej czytelnym przenosząc go to osobnej metody. Dodatkowo mamy "negative condition" oraz metodę z booleanem w argumentach - można zrobić dwa osobne przypadki - addLineContentWithSyntaxColour i druga without syntax.
Find as many defects and problems as possible!