Skip to content

Conversation

@rasheey97-alt
Copy link

I am submitting late, I was have some issues with GitHub " on the pull request dashboard was displaying nothing to compare. so I look at the best solution solve probem and I figure is to delete the assignment from the work space and eclipse.
I delete the folder and start afresh.

Copy link

@abeprosper abeprosper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}

/*

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


import acm.program.*;

public class Hailstone extends ConsoleProgram {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your program does not print out the number of steps it takes to reach 1 as per the assignment requirements. Also your print out statements are hard to read.

* This file is the starter file for the ProgramHierarchy problem.
*/

import acm.graphics.*;

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 (30, 130, 175, 75...etc) throughout your code and this is not a good idea because:

  1. Someone reading your code will be confused how you came up with them
  2. 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.


private static final int HEIGHT = 50;
private static final int WIDTH = 150;
public void run() {

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();
}

}

/*
* File: Pyramid.java

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great solution. Now this is a scalable solution because now when I change BRICK_WIDTH, BRICK_HEIGHT and BRICK_IN_BASE your program still draws a pyramid at the center of the screen.


//Prompt the user for the lengths of the legs,
//and store those two pieces of information in legA and legB.
System.out.println("Please enter the length of the "

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should use ConsoleProgram printing method so this should be
println("Please enter the length of the...") instead of
System.out.println("Please enter the length of the...")


drawLargeRedOval();
drawWhiteOval();
drawSmallRedOval();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great decomposition of the problem using methods.

import acm.graphics.*;
import acm.program.*;
import java.awt.*;

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants