Review #3
Review #3
Conversation
Constants
commit b814f3c Author: 240827 <[email protected]> Date: Tue May 23 23:42:35 2023 +0200 dodanie dokumentacji do klasy DefaultCountingOutRhymer commit b0d0113 Author: 240827 <[email protected]> Date: Tue May 23 23:10:17 2023 +0200 utworzenie metody testRhymers
kowallus
left a comment
kowallus
left a comment
There was a problem hiding this comment.
1 pts (too late penalty)
Specific issues are inlined in the code.
Lack of encapsulation in Node class (Task 11 in "accessory i hermetyzacja")
I suggest starting from the beginning and redo task avoiding mistakes. Fork iis-pio-2023 repo.
There was a problem hiding this comment.
Commits polluted with project files (.settings, .idea, etc.).
| */ | ||
| public class DefaultCountingOutRhymer { | ||
|
|
||
| public static final int NUMBER = 12; |
There was a problem hiding this comment.
Wrong name.
Errors in collaborators work (lack of good review). Extracting constants is done incorrectly. Have you used review hints ('Github - PR checkpoint' at wikamp).
| * @return boolean value indicating if the rhymer is full. | ||
| */ | ||
| public boolean isFull() { | ||
| return total == 11; |
There was a problem hiding this comment.
Another constant. It is related to NUMBER
| * @return boolean value indicating if the rhymer is empty. | ||
| */ | ||
| public boolean isEmpty() { | ||
| return total == NEGATIVE; |
There was a problem hiding this comment.
This constant should be different (separate) than the one in peekaboo and countOut methods.
| @@ -0,0 +1,21 @@ | |||
| package edu.kis.vh.nursery; | |||
|
|
|||
| public class FifoRhymer extends DefaultCountingOutRhymer { | |||
|
|
||
| public class FifoRhymer extends DefaultCountingOutRhymer { | ||
|
|
||
| private DefaultCountingOutRhymer internalRhymer = new DefaultCountingOutRhymer(); |
| public int countOut() { | ||
| while (!isEmpty()) | ||
|
|
||
| internalRhymer.countIn(super.countOut()); | ||
|
|
||
| int ret = internalRhymer.countOut(); | ||
|
|
||
| while (!internalRhymer.isEmpty()) | ||
|
|
||
| countIn(internalRhymer.countOut()); | ||
|
|
There was a problem hiding this comment.
formatting convention issues (problems with merging 'format' branch?)
you forked a wrong repo
| import edu.kis.vh.nursery.defaultCountingOutRhymer; | ||
| import edu.kis.vh.nursery.DefaultCountingOutRhymer; | ||
|
|
||
| public interface Rhymersfactory { |
There was a problem hiding this comment.
Rhymersfactory -> RhymersFactory
| public interface Rhymersfactory { | ||
|
|
||
| public defaultCountingOutRhymer GetStandardRhymer(); | ||
| public DefaultCountingOutRhymer GetStandardRhymer(); |
There was a problem hiding this comment.
Wrong convention. Method names should start from a lower case.
kowallus
left a comment
kowallus
left a comment
There was a problem hiding this comment.
1 pts (too late penalty)
Specific issues are inlined in the code.
Lack of encapsulation in Node class (Task 11 in "accessory i hermetyzacja")
I suggest starting from the beginning and redo task avoiding mistakes. Fork iis-pio-2023 repo.
No description provided.