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

Konstantin_Ivanov#48

Open
kostya23azaza wants to merge 53 commits intomasterfrom
feature/konstantinIvanov
Open

Konstantin_Ivanov#48
kostya23azaza wants to merge 53 commits intomasterfrom
feature/konstantinIvanov

Conversation

@kostya23azaza
Copy link
Collaborator

No description provided.

@kostya23azaza kostya23azaza added the bug Code fix needed label Jul 13, 2021
@NikolaevArtem NikolaevArtem changed the title Feature/konstantin ivanov Konstantin_Ivanov Jul 15, 2021
# Conflicts:
#	README.md
#	src/main/java/homework_1/Main.java
@NikolaevArtem NikolaevArtem added HW_1 Good to go and removed bug Code fix needed labels Jul 15, 2021
@kostya23azaza kostya23azaza added readyForReview Sign for Artem to take a look and removed readyForReview Sign for Artem to take a look labels Jul 18, 2021
import java.util.LinkedList;
import java.util.List;

public final class ImmutableClass {
Copy link
Owner

Choose a reason for hiding this comment

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

Your class is not immutable. I can pass List and then change it, because when you get/set mutable field, you don't make deep copy, you pass the same object (in constructor and in getNew)

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;

public class RandomCharsTableTest extends UnitBase {
Copy link
Owner

Choose a reason for hiding this comment

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

Most of your tests are failing. Please check

Copy link
Owner

Choose a reason for hiding this comment

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

image


import static java.nio.charset.StandardCharsets.UTF_8;

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!

public class Main {

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

Choose a reason for hiding this comment

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

Opt: you could add some greetings message with input example

int result = power.rec(fir, sec);
System.out.println(result);
} catch (Exception e) {
System.out.println("Only 2 non-negative integers are allowed");
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
System.out.println("Only 2 non-negative integers are allowed");
System.out.println(e.getCause());

Opt

Copy link
Owner

Choose a reason for hiding this comment

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

Because you write this message twice in code

if (secondNumber > 0 ) {
return firstNumber * rec(firstNumber, secondNumber-1);
}
return rec(firstNumber, secondNumber) * secondNumber;
Copy link
Owner

Choose a reason for hiding this comment

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

What is the case when this line will be used? You don't test it

Comment on lines 12 to 27
Map<MapProblemsMutableGenerator, String> map = new HashMap<>();
MapProblemsMutableGenerator m1 = new MapProblemsMutableGenerator(1, "name");
MapProblemsMutableGenerator same = new MapProblemsMutableGenerator(1, "name");
MapProblemsMutableGenerator m2 = new MapProblemsMutableGenerator(2, "name2");
MapProblemsMutableGenerator m3 = new MapProblemsMutableGenerator(3, "name3");
MapProblemsMutableGenerator m4 = new MapProblemsMutableGenerator(4, "name4");
map.put(m1, "1");
map.put(m2, "2");
map.put(m3, "3");
map.put(m4, "4");
System.out.println(m1.equals(same));
System.out.println("Trying to get any value by key");
map.get(m1);
map.get(m2);
map.get(m3);
map.get(m4);
Copy link
Owner

Choose a reason for hiding this comment

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

You just override key/value pair, there's no mutable key problem.

Copy link
Owner

Choose a reason for hiding this comment

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

Also, in upper example, collision does appear, but it is not seen in console (opt)

package homework_7;

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.

Code conventions are generally not followed, and Functional interface is created with mistakes (you declare generics, but don't use them)


import static org.junit.jupiter.api.Assertions.assertEquals;

public class CustomAnnotationTest extends UnitBase {
Copy link
Owner

Choose a reason for hiding this comment

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

You can test it all in one test case (opt)

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! Although some changes are needed, and the code needs refactoring according to Code Conventions. Solution with db_annotation is great!

@NikolaevArtem NikolaevArtem added the bug Code fix needed label Sep 20, 2021
@kostya23azaza kostya23azaza added the readyForReview Sign for Artem to take a look label Sep 22, 2021
@NikolaevArtem NikolaevArtem removed the readyForReview Sign for Artem to take a look label Sep 23, 2021
make class final
A parameterized constructor should initialize all the fields performing a deep copy
Deep Copy of objects should be performed in the getter methods
*/public final class Flower {
Copy link
Owner

Choose a reason for hiding this comment

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

Doesn't fit the requirement from Homework char


import java.util.Objects;

public class MapProblemsMutableGenerator {
Copy link
Owner

Choose a reason for hiding this comment

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

Where's mutable key problem?

@NikolaevArtem NikolaevArtem added HW_7 and removed bug Code fix needed labels Sep 23, 2021
@kostya23azaza kostya23azaza added the readyForReview Sign for Artem to take a look label Sep 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

HW_1 Good to go HW_2 HW_4 HW_5 HW_7 readyForReview Sign for Artem to take a look

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants