Add solution for Challenge 10 by shansing#1436
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a new Go package implementing a Shape interface and concrete types (Rectangle, Circle, Triangle) with validated constructors, Area/Perimeter/String methods, and a ShapeCalculator providing aggregate utilities (TotalArea, LargestShape, SortByArea, PrintProperties). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
challenge-10/submissions/shansing/solution-template.go (2)
94-101: Consider extracting semi-perimeter for clarity.The semi-perimeter
(t.SideA + t.SideB + t.SideC) / 2.0is computed four times. Extracting it into a local variable improves readability.♻️ Proposed refactor
func (t *Triangle) Area() float64 { - return math.Sqrt( - ((t.SideA + t.SideB + t.SideC) / 2.0) * - (((t.SideA + t.SideB + t.SideC) / 2.0) - t.SideA) * - (((t.SideA + t.SideB + t.SideC) / 2.0) - t.SideB) * - (((t.SideA + t.SideB + t.SideC) / 2.0) - t.SideC), - ) + s := t.Perimeter() / 2.0 + return math.Sqrt(s * (s - t.SideA) * (s - t.SideB) * (s - t.SideC)) }
140-148: Misleading variable name:lastShapeshould belargestShape.The variable stores the shape with the largest area, not the "last" shape.
♻️ Proposed fix
largestArea := 0.0 - var lastShape Shape + var largestShape Shape for _, shape := range shapes { if shape.Area() > largestArea { largestArea = shape.Area() - lastShape = shape + largestShape = shape } } - return lastShape + return largestShape
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
challenge-10/submissions/shansing/solution-template.go (2)
94-101: Consider extracting the semi-perimeter calculation.The semi-perimeter
(t.SideA + t.SideB + t.SideC) / 2.0is computed four times. Extracting it into a local variable improves readability and adheres to DRY.♻️ Proposed refactor
// Area calculates the area of the triangle using Heron's formula func (t *Triangle) Area() float64 { - return math.Sqrt( - ((t.SideA + t.SideB + t.SideC) / 2.0) * - (((t.SideA + t.SideB + t.SideC) / 2.0) - t.SideA) * - (((t.SideA + t.SideB + t.SideC) / 2.0) - t.SideB) * - (((t.SideA + t.SideB + t.SideC) / 2.0) - t.SideC), - ) + s := t.Perimeter() / 2.0 + return math.Sqrt(s * (s - t.SideA) * (s - t.SideB) * (s - t.SideC)) }
140-148: Misleading variable name:lastShapeshould belargestShape.The variable holds the shape with the largest area found so far, not the "last" shape. Renaming improves code clarity.
📝 Proposed fix
largestArea := 0.0 - var lastShape Shape + var largestShape Shape for _, shape := range shapes { if shape.Area() > largestArea { largestArea = shape.Area() - lastShape = shape + largestShape = shape } } - return lastShape + return largestShape
1 participant
Challenge 10 Solution
Submitted by: @shansing
Challenge: Challenge 10
Description
This PR contains my solution for Challenge 10.
Changes
challenge-10/submissions/shansing/solution-template.goTesting
Thank you for reviewing my submission! 🚀