Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add base classes, add base loop #1734

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

arastorhuiev
Copy link

No description provided.

Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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


Thank you for your hard work on this pull request! 😊 However, there are a few critical issues that need to be addressed before we can approve it:

  1. Incorrect Area Calculations:

    • The formula for calculating the area of a circle is incorrect. It should be Math.PI * radius * radius instead of Math.PI * (radius * 2).
    • The formula for the area of an isosceles trapezoid is also incorrect. It should be ((topLength + bottomLength) * height) / 2 instead of ((bottomLength * topLength) * height) / 0.5.
  2. Figure Type Misassignment: The figure type is incorrectly set to FigureType.RIGHT_RECTANGLE. It should be FigureType.RIGHT_TRIANGLE to accurately reflect the type of figure being represented.

  3. Random Index Calculation:

    • In both FigureSupplier and ColorSupplier, the random index calculation should be corrected to random.nextInt(size) to ensure all items can be selected, including the last one.
  4. Logic for Figure Parameters: The logic for determining which figures should have random parameters and which should have default parameters is not implemented correctly. Please adjust the condition to meet the task requirements.

  5. Code Readability: Avoid passing expressions directly into constructors. Instead, create separate variables for random values and pass them to the constructors for better readability.

Please address these issues to ensure the functionality and correctness of the code. Once these changes are made, we can proceed with the approval. Keep up the great work, and don't hesitate to reach out if you need any help! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨

🔄 Fix the issues, commit the changes, and then re-request my review using the Re-request review button in the Reviewers tab.

src/main/java/core/basesyntax/App.java Outdated Show resolved Hide resolved
src/main/java/core/basesyntax/Circle.java Outdated Show resolved Hide resolved
src/main/java/core/basesyntax/ColorSupplier.java Outdated Show resolved Hide resolved
src/main/java/core/basesyntax/FigureSupplier.java Outdated Show resolved Hide resolved
src/main/java/core/basesyntax/FigureSupplier.java Outdated Show resolved Hide resolved
src/main/java/core/basesyntax/IsoscelesTrapezoid.java Outdated Show resolved Hide resolved
src/main/java/core/basesyntax/RightTriangle.java Outdated Show resolved Hide resolved
src/main/java/core/basesyntax/RightTriangle.java Outdated Show resolved Hide resolved
Copy link

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Great job on implementing the logic for generating and drawing figures! 🎉 However, there are a couple of critical issues that need your attention:

  1. Incorrect Area Calculations:
    • Circle: The area formula should be Math.PI * radius * radius instead of Math.PI * radius * 2.
    • Isosceles Trapezoid: The area formula should be ((topLength + bottomLength) * height) / 2.0 instead of ((bottomLength * topLength) * height) / 2.0.

These corrections are crucial for the accuracy of your calculations. While the decision is to approve this pull request to help you move forward with your learning, please make sure to address these issues in your next iteration. Keep up the good work and continue improving your code! 🚀

✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨


@Override
public double getCalculatedArea() {
return Math.PI * radius * 2;

Choose a reason for hiding this comment

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

The formula for calculating the area of a circle is incorrect. It should be Math.PI * radius * radius instead of Math.PI * radius * 2. Please correct this to ensure the area is calculated accurately.


@Override
public double getCalculatedArea() {
return ((bottomLength * topLength) * height) / 2.0;

Choose a reason for hiding this comment

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

The formula for calculating the area of an isosceles trapezoid is incorrect. It should be ((topLength + bottomLength) * height) / 2.0 instead of ((bottomLength * topLength) * height) / 2.0. Please correct this to ensure the area is calculated accurately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants