Skip to content
This repository was archived by the owner on Jan 18, 2023. It is now read-only.

Zgłoszenie rozwiązania - #2

Open
Wixaxis wants to merge 24 commits into
reviewfrom
master
Open

Zgłoszenie rozwiązania#2
Wixaxis wants to merge 24 commits into
reviewfrom
master

Conversation

@Wixaxis

@Wixaxis Wixaxis commented Jan 25, 2021

Copy link
Copy Markdown
Owner

Przepraszam za spóźnienie :(

@Wixaxis
Wixaxis requested a review from kowallus January 25, 2021 10:35
@Wixaxis

Wixaxis commented Jan 25, 2021

Copy link
Copy Markdown
Owner Author

Javadoc oraz UML do zadania są pod tym linkiem
https://drive.google.com/drive/folders/1lO-Y_GDJvC_p1FEODtbhhsy7jcz7eHU4?usp=sharing

@kowallus kowallus left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2.5 pts

Specific issues inlined in the code.

@@ -0,0 +1,13 @@
<component name="libraryTable">

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commits polluted with project files

public class DefaultCountingOutRhymer {

public static final int DEFAULT_VALUE = -1;
public static final int MAX = 12;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Laconic name

}

public boolean isFull() {
return total == 11;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another constant. It is related to MAX (yet, the name is misinforming)

}

public boolean callCheck() {
return total == -1;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unextracted constant?


protected int peekaboo() {
if (callCheck())
return -1;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another constant


public defaultCountingOutRhymer temp = new defaultCountingOutRhymer();

public DefaultCountingOutRhymer temp = new DefaultCountingOutRhymer();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be private and final

public class HanoiRhymer extends DefaultCountingOutRhymer {

int totalRejected = 0;
int totalRejected = 0;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be private

@Override
public defaultCountingOutRhymer GetStandardRhymer() {
return new defaultCountingOutRhymer();
public DefaultCountingOutRhymer GetStandardRhymer() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

names of methods start from a lowercase (e.g. getStandardRhymer())


class RhymersDemo {

public static final int INT_15 = 15;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

such extracting constants doesn't make sense to me

Comment on lines 5 to 6
final public int value;
public Node prev, next;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about encapsulation?

@Wixaxis

Wixaxis commented Jan 26, 2021

Copy link
Copy Markdown
Owner Author

Added fixes and part of 2.x.x labs

@Wixaxis
Wixaxis requested a review from kowallus January 26, 2021 10:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants