-
Notifications
You must be signed in to change notification settings - Fork 14
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
Solutions to Programming Assignment 2 #8
base: master
Are you sure you want to change the base?
Conversation
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 effort...also why are some of your solutions very similar to Igoche's? Is it just a coincidence...if not you guys should discuss work but when it comes to coding make sure that you code separately that will help improve your skills.
Here is your grade: https://docs.google.com/document/d/13s7ZqN6Q5WBmOs7YSz7Wk4YEz1pAfXeBoyq56JoquYM/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
@@ -10,7 +10,23 @@ | |||
|
|||
public class Hailstone extends ConsoleProgram { |
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 does not work. When I ran the program it kept printing forever and never stopped.
while (x > 1){ | ||
if (x % 2 != 0){ | ||
println ("+ x is odd"); | ||
int y = (3*x) + 1; |
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.
Here you are supposed to update the value of x to 3x + 1 but instead you assigned 3x + 1 to y. Because of this your while loop will never stop since the value of x never changes. Your code should be:
x = (3*x) + 1;
instead of
int y = (3*x) + 1;
println (" so i make 3n+1;" +y); | ||
} else { | ||
println (" +x is even"); | ||
int y = x/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.
Here your code should be:
x = x/2
instead of
int y = x/2
} | ||
} | ||
|
||
import acm.graphics.*; |
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.
Generally you should avoid the use of constant numbers in your program. You have values like (210, 130, 175, 145...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 this 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.
} | ||
} | ||
|
||
import acm.graphics.*; |
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 on this one. Now this is a scalable solution (unlike your solution in ProgramHierarchy.java). Now if we decide to change BRICKS_IN_BASE to 20 (instead of 14) and BRICK_WIDTH to 35 (instead of 30) . Your program still draws the right pyramid without you having to go and change things within the code.
private static final int HEIGHT = 50; | ||
private static final int WIDTH = 150; | ||
|
||
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.
You should breakdown your program into smaller understandable methods. So the run method should have looked like this
public void run() {
drawProgramBox();
drawConsoleLine();
drawConsoleBox();
drawGraphicsLine();
drawGraphicsBox();
drawDialogLine();
drawDialogBox();
}
@@ -10,6 +10,14 @@ | |||
|
|||
public class PythagoreanTheorem 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.
Great work.
/* 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.
The radius and pixel values should have been constants similar to what the starter code for the pyramid question had brick width, brick height and number of bricks on the base as constants. So you should have had something like this:
private static final int PIXELS_PER_INCH = 72;
private static final double RADIUS_OUTER_CIRCLE = 1.0;
private static final double RADIUS_WHITE_CIRCLE = 0.65;
private static final double RADIUS_INNER_CIRCLE = 0.3;
Also again this solution is not scalable (If the assignment requirements changed to have different radiuses your program will not work) and I don't think it uses the right measurements provided by the question (specifically the radiuses: 1.0, 0.65 and 0.3)
my second assignment