Conversation
Walkthrough새로운 Changes
Sequence Diagram(s)sequenceDiagram
participant Widget
participant CustomChartPainter
participant Canvas
Widget->>CustomChartPainter: percentages 전달하여 인스턴스 생성
Widget->>Canvas: paint 메서드 호출
CustomChartPainter->>Canvas: paint(Canvas, Size)
CustomChartPainter->>CustomChartPainter: 세그먼트 각도 및 위치 계산
CustomChartPainter->>Canvas: 세그먼트, 그라디언트, 그림자, 스트로크 그리기
Widget->>CustomChartPainter: shouldRepaint(oldDelegate) 호출
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
frontend/ongi/lib/widgets/custom_chart_painter.dart (3)
17-18: 하드코딩된 값을 설정 가능하게 만드세요
gapAngle과innerRadius비율이 하드코딩되어 있습니다. 더 유연한 차트를 위해 이러한 값들을 생성자 매개변수로 받는 것이 좋습니다.final double gapAngle; final double innerRadiusRatio; CustomChartPainter({ required this.percentages, this.gapAngle = 0.185, this.innerRadiusRatio = 0.65, });
32-33: 그라디언트 색상을 설정 가능하게 만드세요그라디언트 색상이 하드코딩되어 있습니다. 차트의 재사용성을 높이기 위해 색상을 매개변수로 받는 것이 좋습니다.
final List<Color> gradientColors; CustomChartPainter({ required this.percentages, this.gradientColors = const [Color(0xFFFD6C01), Color(0xFFBFECFF)], });
201-201: 주석을 영어로 통일하세요코드베이스의 일관성을 위해 주석을 영어로 작성하는 것이 좋습니다.
-if (len == 0) return p1; // 두 점이 같을 경우 +if (len == 0) return p1; // When two points are the same
| class CustomChartPainter extends CustomPainter { | ||
| final List<double> percentages; | ||
|
|
||
| CustomChartPainter({required this.percentages}); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
입력 검증 추가 필요
percentages 리스트가 비어있거나 유효하지 않은 값을 포함할 수 있습니다. 생성자나 paint 메서드에서 입력 검증을 추가하는 것이 좋습니다.
CustomChartPainter({required this.percentages}) {
assert(percentages.isNotEmpty, 'Percentages list cannot be empty');
assert(percentages.every((p) => p >= 0), 'All percentages must be non-negative');
}🤖 Prompt for AI Agents
In frontend/ongi/lib/widgets/custom_chart_painter.dart at line 7, the
constructor for CustomChartPainter lacks input validation for the percentages
list. Add assertions in the constructor to ensure the percentages list is not
empty and all values are non-negative, preventing invalid data from causing
errors during painting.
| void paint(Canvas canvas, Size size) { | ||
| final Offset center = Offset(size.width / 2, size.height / 2); | ||
| final double outerRadius = size.width / 2; | ||
| final double innerRadius = outerRadius * 0.65; | ||
|
|
||
| final int segmentCount = percentages.length; | ||
| final double totalAngle = 2 * pi; | ||
| final double gapAngle = 0.185; | ||
| final double totalGaps = gapAngle * segmentCount; | ||
|
|
||
| final double totalPercentage = percentages.reduce((a, b) => a + b); | ||
| final List<double> normalizedPercentages = percentages | ||
| .map((p) => p / totalPercentage) | ||
| .toList(); | ||
|
|
||
| final List<double> segmentAngles = normalizedPercentages | ||
| .map((p) => p * (totalAngle - totalGaps)) | ||
| .toList(); | ||
|
|
||
| final LinearGradient gradient = LinearGradient( | ||
| begin: Alignment.topRight, | ||
| end: Alignment.bottomLeft, | ||
| colors: [Color(0xFFFD6C01), Color(0xFFBFECFF)], | ||
| stops: [0.1, 0.9], | ||
| ); | ||
|
|
||
| final shader = gradient.createShader( | ||
| Rect.fromCircle(center: center, radius: outerRadius), | ||
| ); | ||
| final double outerCornerRadius = 10.0; | ||
| final double innerCornerRadius = outerCornerRadius * 0.65; | ||
|
|
||
| Paint shadowPaint = Paint() | ||
| ..color = Colors.black26 | ||
| ..strokeWidth = outerRadius - innerRadius + 35 | ||
| ..style = PaintingStyle.stroke | ||
| ..maskFilter = MaskFilter.blur(BlurStyle.normal, 4.0); | ||
|
|
||
| Paint whitePaint = Paint() | ||
| ..color = Colors.white | ||
| ..strokeWidth = outerRadius - innerRadius + 35 | ||
| ..style = PaintingStyle.stroke; | ||
|
|
||
| Offset shadowOffset = Offset(center.dx, center.dy + 2); | ||
| canvas.drawArc( | ||
| Rect.fromCircle( | ||
| center: shadowOffset, | ||
| radius: (outerRadius + innerRadius) / 2, | ||
| ), | ||
| 0, | ||
| 2 * pi, | ||
| false, | ||
| shadowPaint, | ||
| ); | ||
|
|
||
| canvas.drawArc( | ||
| Rect.fromCircle(center: center, radius: (outerRadius + innerRadius) / 2), | ||
| 0, | ||
| 2 * pi, | ||
| false, | ||
| whitePaint, | ||
| ); | ||
|
|
||
| double currentAngle = -pi / 2 + gapAngle / 2; | ||
|
|
||
| for (int i = 0; i < segmentCount; i++) { | ||
| final double startAngle = currentAngle; | ||
| final double endAngle = startAngle + segmentAngles[i]; | ||
| currentAngle = endAngle + gapAngle; | ||
|
|
||
| Path path = Path(); | ||
|
|
||
| path.arcTo( | ||
| Rect.fromCircle(center: center, radius: outerRadius), | ||
| startAngle, | ||
| segmentAngles[i], | ||
| false, | ||
| ); | ||
|
|
||
| path.arcTo( | ||
| Rect.fromCircle(center: center, radius: innerRadius), | ||
| endAngle, | ||
| -segmentAngles[i], | ||
| false, | ||
| ); | ||
|
|
||
| final Offset startInner = Offset( | ||
| center.dx + innerRadius * cos(startAngle), | ||
| center.dy + innerRadius * sin(startAngle), | ||
| ); | ||
| final Offset startOuter = Offset( | ||
| center.dx + outerRadius * cos(startAngle), | ||
| center.dy + outerRadius * sin(startAngle), | ||
| ); | ||
|
|
||
| final Offset endInner = Offset( | ||
| center.dx + innerRadius * cos(endAngle), | ||
| center.dy + innerRadius * sin(endAngle), | ||
| ); | ||
| final Offset endOuter = Offset( | ||
| center.dx + outerRadius * cos(endAngle), | ||
| center.dy + outerRadius * sin(endAngle), | ||
| ); | ||
|
|
||
| path.moveTo(endOuter.dx, endOuter.dy); | ||
| path.addOval( | ||
| Rect.fromCircle( | ||
| center: getPointAtDistance(endOuter, endInner, outerCornerRadius), | ||
| radius: outerCornerRadius, | ||
| ), | ||
| ); | ||
| path.addOval( | ||
| Rect.fromCircle( | ||
| center: getPointAtDistance(endInner, endOuter, innerCornerRadius), | ||
| radius: innerCornerRadius, | ||
| ), | ||
| ); | ||
|
|
||
| path.addOval( | ||
| Rect.fromCircle( | ||
| center: getPointAtDistance(startOuter, startInner, outerCornerRadius), | ||
| radius: outerCornerRadius, | ||
| ), | ||
| ); | ||
| path.addOval( | ||
| Rect.fromCircle( | ||
| center: getPointAtDistance(startInner, startOuter, innerCornerRadius), | ||
| radius: innerCornerRadius, | ||
| ), | ||
| ); | ||
|
|
||
| Offset ovalEndOuter = moveAlongCircle( | ||
| center: center, | ||
| radius: outerRadius - outerCornerRadius, | ||
| startAngle: endAngle, | ||
| distance: outerCornerRadius, | ||
| ); | ||
| Offset ovalEndInner = moveAlongCircle( | ||
| center: center, | ||
| radius: innerRadius + innerCornerRadius, | ||
| startAngle: endAngle, | ||
| distance: innerCornerRadius, | ||
| ); | ||
|
|
||
| Offset ovalStartOuter = moveAlongCircle( | ||
| center: center, | ||
| radius: outerRadius - outerCornerRadius, | ||
| startAngle: startAngle, | ||
| distance: 0, | ||
| ); | ||
| Offset ovalStartInner = moveAlongCircle( | ||
| center: center, | ||
| radius: innerRadius + innerCornerRadius, | ||
| startAngle: startAngle, | ||
| distance: 0, | ||
| ); | ||
|
|
||
| path.addPath( | ||
| buildSkewedRect( | ||
| ovalEndOuter, | ||
| ovalEndInner, | ||
| outerCornerRadius, | ||
| innerCornerRadius, | ||
| ), | ||
| Offset.zero, | ||
| ); | ||
|
|
||
| path.addPath( | ||
| buildSkewedRect( | ||
| ovalStartOuter, | ||
| ovalStartInner, | ||
| outerCornerRadius, | ||
| innerCornerRadius, | ||
| ), | ||
| Offset.zero, | ||
| ); | ||
| path.close(); | ||
|
|
||
| final Paint segmentPaint = Paint() | ||
| ..shader = shader | ||
| ..style = PaintingStyle.fill; | ||
|
|
||
| canvas.drawPath(path, segmentPaint); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
paint 메서드를 작은 메서드로 분리하세요
paint 메서드가 너무 길고 복잡합니다. 가독성과 유지보수성을 위해 다음과 같이 분리하는 것을 권장합니다:
_drawShadowAndStroke(): 그림자와 흰색 스트로크 그리기_calculateSegmentAngles(): 세그먼트 각도 계산_drawSegment(): 개별 세그먼트 그리기
🤖 Prompt for AI Agents
In frontend/ongi/lib/widgets/custom_chart_painter.dart from lines 10 to 194, the
paint method is too long and complex, reducing readability and maintainability.
Refactor by extracting the shadow and stroke drawing code into a private method
named _drawShadowAndStroke, the segment angle calculations into
_calculateSegmentAngles, and the code that draws each individual segment into
_drawSegment. Then call these helper methods from paint to keep it concise and
clear.
| final double totalPercentage = percentages.reduce((a, b) => a + b); | ||
| final List<double> normalizedPercentages = percentages | ||
| .map((p) => p / totalPercentage) | ||
| .toList(); |
There was a problem hiding this comment.
0으로 나누기 예외 처리 필요
모든 퍼센트가 0인 경우 totalPercentage가 0이 되어 0으로 나누기 오류가 발생할 수 있습니다.
final double totalPercentage = percentages.reduce((a, b) => a + b);
+if (totalPercentage == 0) {
+ return; // 그릴 데이터가 없음
+}
final List<double> normalizedPercentages = percentages
.map((p) => p / totalPercentage)
.toList();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final double totalPercentage = percentages.reduce((a, b) => a + b); | |
| final List<double> normalizedPercentages = percentages | |
| .map((p) => p / totalPercentage) | |
| .toList(); | |
| final double totalPercentage = percentages.reduce((a, b) => a + b); | |
| if (totalPercentage == 0) { | |
| return; // 그릴 데이터가 없음 | |
| } | |
| final List<double> normalizedPercentages = percentages | |
| .map((p) => p / totalPercentage) | |
| .toList(); |
🤖 Prompt for AI Agents
In frontend/ongi/lib/widgets/custom_chart_painter.dart around lines 20 to 23,
the code calculates totalPercentage by summing percentages and then divides each
percentage by totalPercentage without checking if totalPercentage is zero. To
fix this, add a check for totalPercentage being zero before the division; if it
is zero, handle this case appropriately, such as returning a list of zeros or
skipping normalization to avoid division by zero errors.
| final length = sqrt(dx * dx + dy * dy); | ||
|
|
||
| final nx = -dy / length; | ||
| final ny = dx / length; |
There was a problem hiding this comment.
0으로 나누기 예외 처리 필요
두 점이 같은 경우 length가 0이 되어 0으로 나누기 오류가 발생할 수 있습니다.
final length = sqrt(dx * dx + dy * dy);
+if (length == 0) {
+ return Path(); // Return empty path when points are the same
+}
final nx = -dy / length;
final ny = dx / length;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final length = sqrt(dx * dx + dy * dy); | |
| final nx = -dy / length; | |
| final ny = dx / length; | |
| final length = sqrt(dx * dx + dy * dy); | |
| if (length == 0) { | |
| return Path(); // Return empty path when points are the same | |
| } | |
| final nx = -dy / length; | |
| final ny = dx / length; |
🤖 Prompt for AI Agents
In frontend/ongi/lib/widgets/custom_chart_painter.dart around lines 215 to 218,
the calculation divides by length which can be zero if the two points are the
same, causing a divide-by-zero error. Add a check to handle the case when length
is zero before performing the division, for example by returning early or
setting nx and ny to zero or some safe default to avoid the exception.
* feat: 파이 차트 생성하는 CustomPainter 구현 * fix: 리스트 비교 오류 수정
Summary by CodeRabbit