Realization of full application #1280

forzen123212 · 2023-09-27T10:59:00Z

No description provided.

# Conflicts: # src/main/java/core/basesyntax/FigureSupplier.java

mnyshenko · 2023-09-28T08:02:59Z

src/main/java/core/basesyntax/Application.java

+    public static void main(String[] args) {
+        FigureSupplier figureSupplier = new FigureSupplier();
+        GeometricFigure[] figures = new GeometricFigure[FIGURE_COUNT];
+        for (int i = 0; i < figures.length; i++) {


Minor: Since you already have FIGURE_COUNT to represent the length of an array, I suggest using that variable in for expression for better readability

mnyshenko · 2023-09-28T08:06:56Z

src/main/java/core/basesyntax/Color.java

+public enum Color {
+    BLACK, WHITE, YELLOW, GREEN, BLUE, ORANGE, PURPLE, PINK
+}


It's better practice to put each value on a new line

mnyshenko · 2023-09-28T08:07:38Z

src/main/java/core/basesyntax/ColorSupplier.java

+        int index = random.nextInt(Color.values().length);
+        Color color = Color.values()[index];


I suggest extracting Color.values() on a class level for further reuse of that variable

mnyshenko · 2023-09-28T08:09:14Z

src/main/java/core/basesyntax/FigureSupplier.java

+import java.util.Random;
+
+public class FigureSupplier {
+    private static final ColorSupplier COLOR_SUPPLIER = new ColorSupplier();


Each FigureSupplier should have its own ColorSupplier, it should not be a constant

mnyshenko · 2023-09-28T08:14:09Z

src/main/java/core/basesyntax/GeometricFigure.java

+    public GeometricFigure(String colour) {
+        this.color = colour;
+    }


minor: please rename colour -> color

mnyshenko · 2023-09-28T08:15:48Z

src/main/java/core/basesyntax/FigureSupplier.java

+    private static final int MAX_LENGTH = 100;
+    private static final int FIGURE_COUNT = 5;
+    private static final int DEFAULT_RADIUS = 10;
+    private static final String DEFAULT_COLOR = String.valueOf(Color.GREEN);


Minor: as the task says the default color should be white

mnyshenko · 2023-09-28T08:17:43Z

src/main/java/core/basesyntax/GeometricFigure.java

@@ -0,0 +1,17 @@
+package core.basesyntax;
+
+public abstract class GeometricFigure implements Drawable, AreaCalculator {


Please pay attention to the task requirements which ask you to create a Figure class, not GeometricFigure

mnyshenko · 2023-09-28T08:19:14Z

src/test/java/core/test/StructureTest.java

I guess you're not supposed to change this file

mnyshenko · 2023-09-28T08:26:25Z

src/main/java/core/basesyntax/FigureSupplier.java

+        switch (index) {
+            case 0:
+                return new Circle(randomProperty, randomColor);
+            case 1:
+                return new IsoscelesTrapezoid(randomProperty, randomProperty,
+                        randomProperty, randomColor);
+            case 2:
+                return new Rectangle(randomProperty, randomProperty, randomColor);
+            case 3:
+                return new RightTriangle(randomProperty, randomProperty, randomColor);
+            default:
+                return new Square(randomProperty, randomColor);
+        }


It seems not quite the right implementation. For example, you will always pass 2 equal numbers to the Rectangle constructor, so you'll always get a Square actually. Similar problems occur for other figures

d1sam

Good job, but some changes should be done.

d1sam · 2023-10-02T07:37:33Z

src/main/java/core/basesyntax/Figure.java

+    public String getColor() {
+        return color;
+    }
+


Please don't add this empty redundant line.

d1sam · 2023-10-02T07:38:51Z

src/main/java/core/basesyntax/ColorSupplier.java

+
+public class ColorSupplier {
+    private final Random random = new Random();
+    private final Color[] colorSet = Color.values();


Please choose better name for this variable, because, it's not a Set, it's an array :) You will learn about Set in the nearest future.

d1sam · 2023-10-02T07:40:31Z

src/test/java/core/test/StructureTest.java

Please don't make changes inside test package.

d1sam · 2023-10-02T07:43:07Z

src/main/java/core/basesyntax/FigureSupplier.java

+        if (random.nextInt(MAX_LENGTH) == 0) {
+            return random.nextInt(MAX_LENGTH);
+        }
+        return random.nextInt(MAX_LENGTH);


Please simplify this.
The best solution is to just always add 1 and request random numbers from a smaller range:
rand.random(MAX_LENGTH - 1) + 1;

d1sam · 2023-10-02T07:44:01Z

src/main/java/core/basesyntax/FigureSupplier.java

+    private static final int MAX_LENGTH = 100;
+    private static final int FIGURE_COUNT = 5;
+    private static final int DEFAULT_RADIUS = 10;
+    private static final String DEFAULT_COLOR = String.valueOf(Color.WHITE);


Use name() for getting String representation of enum constants
Don’t use toString() or String.valueOf()(it will call toString() under the hood) for getting String representation of enum constants. toString() is common for all enum constants. If you override this method like below:

@Override public String() toString() { return "default"; }

then for every constant toString() will be returning default, that’s not ok. So it’s better to use standard method of enum name() that will be returning always String representation of concrete enum constant.

bhdnchui

Good job, just pay attention to my comments.

bhdnchui · 2023-10-03T07:07:42Z

src/test/java/core/test/StructureTest.java

@@ -16,7 +16,7 @@

 public class StructureTest {
  private static final List<String> figureClassNames = List
-      .of("Circle", "Square", "IsoscelesTrapezoid", "Rectangle", "RightTriangle");
+          .of("Circle", "Square", "IsoscelesTrapezoid", "Rectangle", "RightTriangle");


I think you should only provide a solution and not change tests.

bhdnchui · 2023-10-03T07:07:47Z

src/test/java/core/test/StructureTest.java

+                + "and path to project and project name should not contain spaces "
+                + "or words in cyrillic");


I think you should only provide a solution and not change tests.

bhdnchui · 2023-10-03T07:07:59Z

src/test/java/core/test/StructureTest.java

@@ -152,4 +152,4 @@ private static List<Class> findClasses(File directory, String packageName)
    }
    return classes;
  }
-}
+}


The same as above.

forzen123212 added 8 commits September 27, 2023 13:58

Realization of full application

b7ef090

Realization of full application

515b89f

Merge remote-tracking branch 'origin/ForzenBranch' into ForzenBranch

cae4fac

# Conflicts: # src/main/java/core/basesyntax/FigureSupplier.java

Realization of full application

51c56c3

Realization of full application

5e19cc3

Realization of full application

7fa69f2

Merge remote-tracking branch 'origin/ForzenBranch' into ForzenBranch

24686a8

Realization of full application

cd42fa0

mnyshenko suggested changes Sep 28, 2023

View reviewed changes

forzen123212 added 3 commits September 28, 2023 18:35

Fixed style and main method

a50dbd1

Fixed style and main method

f3d8d66

Fixed style and main method

8223f8c

forzen123212 requested a review from mnyshenko September 28, 2023 16:28

d1sam suggested changes Oct 2, 2023

View reviewed changes

Added fixes

797aaf3

forzen123212 requested a review from d1sam October 2, 2023 07:51

bhdnchui approved these changes Oct 3, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Realization of full application #1280

Realization of full application #1280

forzen123212 commented Sep 27, 2023

mnyshenko Sep 28, 2023

mnyshenko Sep 28, 2023

mnyshenko Sep 28, 2023

mnyshenko Sep 28, 2023

mnyshenko Sep 28, 2023

mnyshenko Sep 28, 2023

mnyshenko Sep 28, 2023

mnyshenko Sep 28, 2023

mnyshenko Sep 28, 2023

d1sam left a comment

d1sam Oct 2, 2023

d1sam Oct 2, 2023

d1sam Oct 2, 2023

d1sam Oct 2, 2023

d1sam Oct 2, 2023

bhdnchui left a comment

bhdnchui Oct 3, 2023

bhdnchui Oct 3, 2023

bhdnchui Oct 3, 2023

		int index = random.nextInt(Color.values().length);
		Color color = Color.values()[index];

		@@ -0,0 +1,17 @@
		package core.basesyntax;

		public abstract class GeometricFigure implements Drawable, AreaCalculator {

		+ "and path to project and project name should not contain spaces "
		+ "or words in cyrillic");

Realization of full application #1280

Are you sure you want to change the base?

Realization of full application #1280

Conversation

forzen123212 commented Sep 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d1sam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bhdnchui left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment