Skip to content
This repository was archived by the owner on Oct 25, 2021. It is now read-only.

Tatiana_Dzenzura#34

Open
tmandreeva1 wants to merge 35 commits intomasterfrom
feature/TatianaDzenzura
Open

Tatiana_Dzenzura#34
tmandreeva1 wants to merge 35 commits intomasterfrom
feature/TatianaDzenzura

Conversation

@tmandreeva1
Copy link
Collaborator

No description provided.

@tmandreeva1 tmandreeva1 added the readyForReview Sign for Artem to take a look label Jul 10, 2021
@NikolaevArtem NikolaevArtem added bug Code fix needed and removed readyForReview Sign for Artem to take a look labels Jul 11, 2021
@NikolaevArtem NikolaevArtem changed the title feature/TatianaDzenzura Tatiana_Dzenzura Jul 11, 2021
@tmandreeva1 tmandreeva1 added the readyForReview Sign for Artem to take a look label Jul 13, 2021
@NikolaevArtem NikolaevArtem added HW_1 Good to go and removed bug Code fix needed readyForReview Sign for Artem to take a look labels Jul 13, 2021
@tmandreeva1 tmandreeva1 added the readyForReview Sign for Artem to take a look label Jul 15, 2021
@NikolaevArtem NikolaevArtem removed the readyForReview Sign for Artem to take a look label Jul 20, 2021
@NikolaevArtem
Copy link
Owner

Good! All tests pass, the apps work as expected. There's small comment to fix

Comment on lines +66 to +67
ClassLoader classLoader = getClass().getClassLoader();
return new File(Objects.requireNonNull(classLoader.getResource(fileName)).getFile());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ClassLoader classLoader = getClass().getClassLoader();
return new File(Objects.requireNonNull(classLoader.getResource(fileName)).getFile());
return new File(pathToResources + fileName);


public class Main {
public static void main(String[] args) {
new GameLauncher().launch();
Copy link
Owner

Choose a reason for hiding this comment

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

It's pretty uncomfortable to play... especially to put ships. Also, what rules did you use? I strongly remember there should be 1 4-cells ship and 4 1-cell ships...

Copy link
Owner

Choose a reason for hiding this comment

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

CARRIER(5),
BATTLESHIP(4),
CRUISER(3),
SUBMARINE(3),
DESTROYER(2);

From your code

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Rules: https://www.hasbro.com/common/instruct/Battleship.PDF

Also, we were free to choose ui, as I remember

Copy link
Owner

Choose a reason for hiding this comment

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

In case of wrong input app shuts down...
image
Hitting the same cell is allowed

Copy link
Owner

Choose a reason for hiding this comment

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

image

Copy link
Owner

Choose a reason for hiding this comment

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

Interface is nice though, but the app is buggy. I'm sorry I can't accept that

@NikolaevArtem NikolaevArtem added bug Code fix needed and removed readyForReview Sign for Artem to take a look labels Sep 23, 2021
@tmandreeva1 tmandreeva1 added the readyForReview Sign for Artem to take a look label Sep 23, 2021
Comment on lines +4 to +8
public static void main(String[] args) {
new CustomFileReader().run1();
new CustomFileReader().run2();
new CustomFileReader().run3();
}
Copy link
Owner

Choose a reason for hiding this comment

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

Is it expected?
image

public class Main {

public static void main(String[] args) {
new PowerOfNumber().run();
Copy link
Owner

Choose a reason for hiding this comment

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

image

@NikolaevArtem NikolaevArtem added HW_5 HW_6 HW_7 and removed readyForReview Sign for Artem to take a look labels Sep 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants