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

Ilia_Prokofev#15

Open
blowushka wants to merge 72 commits intomasterfrom
feature/IliaProkofev
Open

Ilia_Prokofev#15
blowushka wants to merge 72 commits intomasterfrom
feature/IliaProkofev

Conversation

@blowushka
Copy link
Collaborator

Completed homework_1

@NikolaevArtem NikolaevArtem added the bug Code fix needed label Jul 9, 2021
@blowushka blowushka added the readyForReview Sign for Artem to take a look label Jul 9, 2021
@NikolaevArtem NikolaevArtem removed the readyForReview Sign for Artem to take a look label Jul 9, 2021
@NikolaevArtem NikolaevArtem removed bug Code fix needed readyForReview Sign for Artem to take a look labels Sep 22, 2021

public class Main {

public static void main(String[] args) {
Copy link
Owner

Choose a reason for hiding this comment

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

Good job! Nice interface, a lot of features (such as automatic ship placement, different game modes), comfortable to play, informative messages. I like the architecture, the code is readable and understandable. You show good knowledge of Java Core (missing Java 8 and some concepts of OOP).

To make it better: add automatic miss marks placement around a destroyed ship. Also, your code has a lot of static. It seems logical that if you don't care about having multiple objects of a class in your app (like Ship), then mark it static, but actually in this case it should be a singleton, and still dynamic. Static is rarely used in Java, mostly for constants and utils. And mostly your methods are void, which is also rare.

Approved!

Comment on lines +38 to +40
} catch (NoSuchMethodException e) {
temp = "Aristarch";
this.age = 50;
Copy link
Owner

Choose a reason for hiding this comment

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

opt: That's unexpected.. in catch clause you should handle error, not hide it and surely not insert some "magic" data

//this.name = name;
this.age = age;
} catch (NoSuchMethodException e) {
name = "Aristarch";
Copy link
Owner

Choose a reason for hiding this comment

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

opt: here you put value for local var, not for class field

import java.util.List;
import java.util.Scanner;

public class CustomFileReader {
Copy link
Owner

Choose a reason for hiding this comment

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

Good!

Copy link
Owner

@NikolaevArtem NikolaevArtem left a comment

Choose a reason for hiding this comment

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

Good job! Except for minor fixes in HW_3 and HW_6, homework is done! Today on the lecture we will discuss these homework, as somehow many students have troubles with these

@blowushka blowushka added the readyForReview Sign for Artem to take a look label Sep 22, 2021
@NikolaevArtem NikolaevArtem added HW_3 HW_6 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants