Lab1 #37
Lab1 #37
Conversation
|
Dziękuję. Uprzejmię proszę jeszcze o maila ze zgłoszeniem tego PR (link), zgodnie z zasadami opisanym w pliku readme.md w repozytorium. |
|
Specific issues inlined in the code. Required updates are marked using 'PLEASE FIX:' prefix. In case of repeated issues usually only one place is marked, but fixes are expected to be made to similar places. |
| @@ -0,0 +1,13 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
There was a problem hiding this comment.
Commits polluted with project/compilation files
|
|
||
| public class DefaultCountingOutRhymer { | ||
|
|
||
| private static final int full = 11; |
There was a problem hiding this comment.
PLEASE FIX: Constants names should be written UPPERCASE (e.g. FULL).
The name is unclear (possibly incorrect).
|
|
||
| protected int peekaboo() { | ||
| if (callCheck()) | ||
| return wrongVal; |
There was a problem hiding this comment.
DEFAULT_VALUE or EMPTY_STACK_VALUE would be better
| private static final int wrongVal = -1; | ||
| private final int[] NUMBERS = new int[12]; | ||
|
|
||
| private int total = wrongVal; |
There was a problem hiding this comment.
This is not a WRONG_VALUE (wrong name)
|
|
||
| private static final int full = 11; | ||
| private static final int wrongVal = -1; | ||
| private final int[] NUMBERS = new int[12]; |
There was a problem hiding this comment.
Array is not a constant (lowercase)
Another constant (12). It is related to FULL (yet, the name is misinforming)
| return new defaultCountingOutRhymer(); | ||
| } | ||
| @Override | ||
| public DefaultCountingOutRhymer GetStandardRhymer() { |
There was a problem hiding this comment.
PLEASE FIX: Method names should start from lowercase (e.g. getStandardRhymer())
| @@ -2,11 +2,35 @@ | |||
|
|
|||
| public class Node { | |||
|
|
|||
| public int value; | |||
| public Node prev, next; | |||
| private int value; | |||
There was a problem hiding this comment.
PLEASE FIX: Immutable - should be final (setter unnecessary)
| return ret; | ||
| } | ||
| private Node last; | ||
| private int i; |
There was a problem hiding this comment.
PLEASE FIX: Unused private field (to be removed).
| private static DefaultCountingOutRhymer[] rhymersFactory() { | ||
| Rhymersfactory factory = new DefaultRhymersFactory(); | ||
|
|
||
| return new DefaultCountingOutRhymer[]{factory.GetStandardRhymer(), factory.GetFalseRhymer(), |
There was a problem hiding this comment.
PLEASE FIX: This part should remain in main function. The rest (for loops) should be extracted to testRhymers method (factory should be its parameter)
| @@ -0,0 +1,36 @@ | |||
| <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd"> | |||
There was a problem hiding this comment.
JavaDoc documents should not be generated, only JavaDoc java comments. (JavaDoc should be generated from sources outside the repo.)
Dzień dobry,
Przepraszam że tak późno, ale chciałbym oddać zadania na pierwsze laboratoria.