Skip to content

Conversation

@hedreez09
Copy link

So sorry i couldn't submit on time it was due to bad network service in house.
Pls pardon me for my wrong deeds.

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.


import acm.program.*;

public class FindRange extends ConsoleProgram {

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.

I suppose you ran out of time to work on this. :(

}
}

/*

Choose a reason for hiding this comment

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

Your solution does not draw the diagram at the center of the screen.

Also generally you should avoid the use of constant numbers in your program. You have values like (58 , 178, 249, 230...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.

}
}

/*

Choose a reason for hiding this comment

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

Excellent solution. Now this is a scalable solution. If I change BRICKS_IN_BASE, BRICK_WIDTH and BRICK_HEIGHT values still the program draws the pyramid correctly at the center.

/* You fill this in */
}
}
/*

Choose a reason for hiding this comment

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

Great solution.

import acm.program.*;

public class FindRange extends ConsoleProgram {
private static final int Sentinel = 0;

Choose a reason for hiding this comment

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

the convention for naming static variables is all caps. So SENTINEL instead of Sentinel

/* You fill this in. */
}
}
/*

Choose a reason for hiding this comment

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

This implementation does not work. I did not see the target being drawn when I ran the code.

public class Target extends GraphicsProgram {


public void run(double OUTTERRad) {

Choose a reason for hiding this comment

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

I don't think you should be passing an argument to the run method. Also you should follow naming convention. You can't have ALL caps mixed with non small letters. So you can't have this:
OUTTERRad or outterRAD...
Your argument should be: outterRad (This naming convention is called Camel case naming convention).


GOval INNERr =new GOval(x - INNERRad,y - INNERRad,INNERRad *2,INNERRad*2 );
INNERRad = INNER*inchToPixel;
GOval s = new GOval (80,60,40,40);

Choose a reason for hiding this comment

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

You should avoid having random values, someone reading would not know why you chose 80, 60, and 40 here. Also you should choose proper variable names, "s" is not a good variable name. For example:
GOVal middleCircle = new GOVal() is better. I know here you are declaring a middle circle.
On the other hand:
GOVal s = new GOVal() is confusing cause I don't know what s is.

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