Solution#3329
Conversation
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation has two critical issues that violate explicit requirements. First, Triangle's getArea() method returns the full precision value from Heron's formula instead of rounding down to hundredths like Circle does. You need to use Math.floor(result * 100) / 100 to match the expected output format. Second, all your error messages simply say 'Invalid length' which violates checklist item #3 requiring detailed error messages. Each error should clearly explain what failed - for example, 'Side a must be a positive number' for non-positive sides, and 'Sides [a], [b] and [c] cannot form a triangle because the longest side must be less than the sum of the other two sides' for triangle inequality violations. These are not cosmetic issues - they are explicit requirements from the task checklist that must be addressed.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
| if (a <= 0 || b <= 0 || c <= 0) { | ||
| throw new Error('Invalid length'); |
There was a problem hiding this comment.
This violates checklist item #3: error messages should be detailed. For example, 'Side length must be a positive number' or 'Sides 1, 2 and 3 cannot form a triangle' (as shown in the task description). Generic 'Invalid length' is not descriptive enough.
|
|
||
| if (max === a) { | ||
| if (max >= b + c) { | ||
| throw new Error('Invalid length'); | ||
| } | ||
| } else if (max === b) { | ||
| if (max >= a + c) { | ||
| throw new Error('Invalid length'); | ||
| } | ||
| } else { | ||
| if (max >= a + b) { | ||
| throw new Error('Invalid length'); |
There was a problem hiding this comment.
This violates checklist item #3: the error message 'Invalid length' doesn't explain the triangle inequality violation. Consider a message like 'Sides [a], [b] and [c] cannot form a triangle because the longest side is greater than or equal to the sum of the other two sides'.
| } else if (max === b) { | ||
| if (max >= a + c) { | ||
| throw new Error('Invalid length'); |
There was a problem hiding this comment.
This violates checklist item #3: similar issue with generic 'Invalid length'. The message should distinguish this case from the non-positive side case above.
| public getArea(): number { | ||
| const s = 0.5 * (this.a + this.b + this.c); | ||
|
|
||
| return Math.sqrt(s * (s - this.a) * (s - this.b) * (s - this.c)); |
There was a problem hiding this comment.
Triangle's getArea() should round down to hundredths like Circle's getArea() does. Consider using Math.floor(result * 100) / 100 to match the expected output format (e.g., for sides 3, 4, 5 the result should be 6, not 6.000000000000001).
| if (radius <= 0) { | ||
| throw new Error('Invalid length'); |
There was a problem hiding this comment.
This violates checklist item #3: consider a more descriptive error like 'Circle radius must be a positive number' instead of generic 'Invalid length'.
| if (width <= 0 || height <= 0) { | ||
| throw new Error('Invalid length'); |
There was a problem hiding this comment.
This violates checklist item #3: consider more descriptive errors like 'Rectangle width must be a positive number' and 'Rectangle height must be a positive number' instead of generic 'Invalid length'.
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
I'm requesting changes for this implementation. While most of the code is working correctly, there are two critical issues that need to be addressed: 1. Rectangle's getArea() is missing the required floor-to-hundredths rounding. The requirement explicitly states all figures must round down to hundredths, and Circle/Triangle already implement this correctly. Rectangle currently returns this.width * this.height with full precision. You need to add the rounding: Math.floor(this.width * this.height * 100) / 100. 2. getInfo() uses .toFixed(2) which corrupts correctly rounded values. Since getArea() now returns properly floored values (e.g., 9.277), using .toFixed(2) will round them back up (9.28), violating the requirement. For example, if getArea() returns 9.277, the output becomes "9.28" instead of the required "9.27". Simply use the value directly in the template literal: A ${figure.color} ${figure.shape} - ${figure.getArea()} without any .toFixed() call. Once you fix these two rounding issues, your implementation should pass all requirements. The Triangle fix and detailed error messages look great—nice work on those improvements!
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Important
Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.
Tip
If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.
Caution
Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.
Footnotes
|
|
||
| public getArea(): number { | ||
| return this.width * this.height; |
There was a problem hiding this comment.
Rectangle's getArea() doesn't round down to hundredths like Circle and Triangle do. For consistency with the requirement 'returns the area rounded down to hundredths' and to match Circle's implementation, consider using: Math.floor(this.width * this.height * 100) / 100
| export function getInfo(figure: Figures): string { | ||
| return `A ${figure.color} ${figure.shape} - ${+figure.getArea().toFixed(2)}`; |
There was a problem hiding this comment.
Using .toFixed(2) here can cause incorrect rounding. For example, if getArea() returns 9.2775 (already rounded down), .toFixed(2) would round it up to 9.28. Since getArea() now returns properly rounded values, you could use String(figure.getArea()) or the template literal directly without .toFixed().
mateacademy-ai-mentor
left a comment
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your solution is approved. All three figure classes are correctly implemented with proper validation, the Figure interface is correctly defined with a parameterless getArea() method, and Rectangle's area calculation properly floors to hundredths as required. The triangle inequality check for Heron's formula, circle's use of Math.PI, and detailed error messages all meet the requirements. Well done on successfully addressing the previous feedback about the Rectangle class - your implementation is clean and follows the specifications precisely.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
2 participants
No description provided.