-
Notifications
You must be signed in to change notification settings - Fork 14
Programming Assignment 2 Solutions. #5
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
base: master
Are you sure you want to change the base?
Conversation
abeprosper
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great attempt. Keep on improving. Here is the link to your grade:
https://docs.google.com/document/d/1DGn91Q5PNu2WF2eKPpdpuDz4b_Ecg1dqOFeW90-hiLw/edit?usp=sharing
| } | ||
| } | ||
|
|
||
| /* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
| public class Hailstone extends ConsoleProgram { | ||
| public void run() { | ||
| /* You fill this in */ | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work.
| import acm.program.*; | ||
| import java.awt.*; | ||
|
|
||
| public class ProgramHierarchy extends GraphicsProgram { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your solution doesn't work according to the program requirements. Your diagram is not centered on the screen as per the assignment description in the last bulletin: "The entire figure should be centered in the window"
Also generally you should avoid the use of constant numbers in your program. You have values like (288, 130, 450, 150...etc) throughout your code and this is not a good idea because:
- Someone reading your code will be confused how you came up with them
- If requirements change your program will be really hard to update. For example imagine for this exercise the requirements changed and we decide that the value for HEIGHT (50) and WIDTH (150) should be changed to HEIGHT = 60 and WIDTH = 180
If the second thing mention above happens your program will fail to draw the right diagram. To make it work you will have to go back and start changing all the numbers throughout your program. It should be possible to come up with a solution that works without you doing further modification. The ability of a program to be able to adapt to changes like that is called scalability. So in this case your solution is not scalable enough.
|
|
||
| public class ProgramHierarchy extends GraphicsProgram { | ||
| public void run() { | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The assignment description said:
The width and height of the class boxes should be specified as named constants so that they are easy to change.
So your program should have them as constants as:
private static final int HEIGHT = 50;
private static final int WIDTH = 150;
|
|
||
|
|
||
| public void run() { | ||
| double x = (getWidth() - WIDTH); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The calculations for x and y here are incorrect, they should have instead been:
double x = (getWidth() - BRICKS_IN_BASE * BRICK_WIDTH) / 2;
double y = getHeight() - BRICK_HEIGHT;
|
|
||
|
|
||
| // | ||
| println("this is the sqare root of:" + c1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output here should not be "this is the square root of: " instead your line should have been:
println ("c:" + c); as per the problem description.
| */ | ||
|
|
||
| import acm.program.*; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work.
| * File: Target.java | ||
| * Name: | ||
| * Section Leader: | ||
| * ----------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a great attempt because this solution is scalable. If the user decide to modify the radius of the outer circle for example, you wouldn't need to in your code and try to modify it. Instead it will just do the right thing.
However, your solution doesn't position the target at the center. You should have specified where the circles should be drawn in your "add" method.
| double innerRad = 0.3 * InchesToRadius; | ||
| double centerX = getWidth()/2; | ||
| double centerY = getHeight()/2; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The values for the center (centerX and centerY) should be calculated within the run method instead of here. Also constants are declared as follows:
private static final int INCHES_TO_RADIUS = 72;
private static final double OUTER_RAD = 1.0 * INCHES_TO_RADIUS;
private static final double MIDDLE_RAD = 0.65 * INCHES_TO_RADIUS;
private static final double INNER_RAD = 0.3 * INCHES_TO_RADIUS;
| innerCircle.setColor(Color.RED); | ||
| innerCircle.setFillColor(Color.RED); | ||
| innerCircle.setFilled(true); | ||
| add (innerCircle); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should have defined a method for the three code chunks and call it with the right parameters instead of repeating the same code 3 times. So the method could have looked roughly something like this:
private void drawCircle(double centerX, double centerY, double rad, Color c) {
GOval circle = new GOval(centerX - rad, centerY - rad, 2*rad, 2*rad);
circle.setColor(c);
circle.setFillColor(c);
circle.setFilled (true);
add (circle);
}
this contain the programming assignment 2 solutions. would have uploaded it since yesterday but light issue in ma area. thanks