Abwas submitting his assignment 2 #4
Conversation
abeprosper
left a comment
abeprosper
left a comment
There was a problem hiding this comment.
Amazing work. Significant improvement from your last assignment. Here is your grade:
https://docs.google.com/document/d/1wMLx8djV3-2gYw5HRGzqgE1ZTF4TOo-dSb4rPBReL7U/edit?usp=sharing
| } | ||
| } | ||
|
|
||
| /* |
There was a problem hiding this comment.
Program doesn't work accurately. It fails the following two cases described in the assignment:
If the user enters only one value before the sentinel, the program should report
that value as both the largest and smallest
So for example if someone enters 5 followed by 0 the program should report:
smallest: 5
largest: 5
Your program instead reports
smallest: 0
largest: 5
Also
If the user enters the sentinel on the very first input line, then no values have been
entered, and your program should display a message to that effect.
So if for example the user enters 0 the program should report:
No values has been entered.
Your program instead reports:
Smallest: 0
Largest: 0
| } | ||
| } | ||
|
|
||
| /* |
| * This file is the starter file for the ProgramHierarchy problem. | ||
| */ | ||
|
|
||
| import acm.graphics.*; |
There was a problem hiding this comment.
Great solution and good job on breaking down your code into different methods. You also did a good job on making sure that your code is scalable (Still works even when BOX_WIDTH and BOX_HEIGHT values are changed). However, while your program handles changes to BOX_WIDTH well, it does not do well when I change BOX_HEIGHT. Still great work overall.
| import acm.program.*; | ||
| import java.awt.*; | ||
|
|
||
| public class Pyramid extends GraphicsProgram { |
There was a problem hiding this comment.
Great solution, however again your program is not completely scalable. If I change BRICK_WIDTH or BRICKS_IN_BASE, your solution still draws the pyramid however it is no longer centered on the screen. This is because your calculation for the x starting position is not accurately done. However great solution overall.
|
|
||
| public void run() { | ||
|
|
||
| int x = getWidth() / 4; |
There was a problem hiding this comment.
This should instead be:
double x = (getWidth() - BRICKS_IN_BASE * BRICK_WIDTH) / 2;
| /* You fill this in */ | ||
| } | ||
| } | ||
| /* |
| /* You fill this in. */ | ||
| } | ||
| } | ||
| /* |
There was a problem hiding this comment.
Great solution. This solution is completely scalable since whenever I change the value of the outer, middle or inner radius still I get a correct target without doing any further modification to the code.
|
|
||
| double middleRad = 0.65 * inchToPixel; | ||
|
|
||
| double innerRad = 0.30 * inchToPixel; |
There was a problem hiding this comment.
You should declare these values as constants outside the run method as shown below:
private static final int PIXELS_PER_INCH = 72;
private static final double RADIUS_OUTER_CIRCLE = 1.0 * PIXELS_PER_INCH ;
private static final double RADIUS_WHITE_CIRCLE = 0.65 * PIXELS_PER_INCH;
private static final double RADIUS_INNER_CIRCLE = 0.30 * PIXELS_PER_INCH;
| innerCircle.setColor(Color.RED); | ||
|
|
||
| add(innerCircle); | ||
|
|
There was a problem hiding this comment.
The outerCircle, middlerCircle and innerCircle drawing should have been done in one method that way you would not be repeating very similar code 3 times. So for example you could have defined a method called drawCircle as follows:
private void drawCircle(double centerX, double centerY, double r, Color c) {
GOval circle = new GOval (centerX - r, centerY - r, 2 * r, 2 * r);
circle.setFilled(true);
circle.setColor(c);
add(circle);
}
And in the run method you would have called it as so:
drawCircle(centerX, centerY, outerRad, Color.RED)
drawCircle(centerX, centerY, middleRad, Color.WHITE)
drawCircle(centerX, centerY, innerRad, Color.RED)
2 participants
No description provided.