Need help with Java Homework

[CreepyPro]

I need to write a method for the Clock class that advances time. The problem is that I'm not passing all my test cases and I can't figure out why, so if someone could explain to me what I'm doing wrong that would be great.

I do not need the answer for the exercise, just to know why my code doesn't work.

 

Clock class + exercise: https://codestepbystep.com/problemset/view/1348

 

My code:

public void advance(int a){
    int m =a%60; //Minutes to add
    int h =a/60; //Hours to add
    
    minute = minute + m;
    while(minute>60) {
        minute = minute - 60;
        hour++;
    }
    hour = hour + h;
    while(hour>12) {
        if(getAmPm()=="AM") {
            amPm = "PM";
        } else
            amPm = "AM";
        hour = hour - 12;
    }
}       

 

[Bacon soup]

public void advance(int a){
    int m =a%60; //Minutes to add 
    int h =a/60; //Hours to add
    //you are calculating the hours to add here but then you also do the while loop below?
    
    minute = minute + m; 
    //the below while loop can be replaced with a simple arithmatic equation, like say hour += a/60;
    while(minute>60) {
        minute = minute - 60;
        hour++;
    } 
    hour = hour + h; 
//There is clearly too much going on here. If you are using the above to change a global variable you should replace all above with a setter method
e.g. 
incrementMinute(a);
incrementHour(a);

    
    while(hour>12) {
        if(getAmPm()=="AM") { 
            amPm = "PM";
        } else
            amPm = "AM";
        hour = hour - 12; <-  
    }
}       
//where is this getAmPm method? This while loop is weird and you need to clean it up. Can you ditch the while loop and getAmPm() and just use an if statement asking if hour is > 12? e.g. 
  String amPm = (getHour() > 12) ? "PM" : "AM";
  setAmPm(amPm);

[wanderingfool2]

@Bacon soup Yes there can be a lot cleaner/more optimized ways to implement this, but given that he is learning having things like loops could be beneficial in learning; and you didn't really mention why his was failing.  (Actually as a note, the arithmetic way of doing it would maybe look at bit more menacing if you don't take into consideration all the cases.  An example being your amPm bit...if the getHour is 25 it would set it to PM even though it should be AM.  Actually fundamentally, I think CreepyPro did an okay job in an initial thought outness.  (To your comment about adding the hours, it is more about accounting for the rollover of the hour, so he didn't double adding)

 

@CreepyPro So in your case it is important to think about boundary conditions.

Consider the minute>60 line.  minute = 60 will be an issue (You time may be 1:60AM )  Other than that, it looks maybe fine.

[jslowik]

3 hours ago, Bacon soup said:

String amPm = (getHour() > 12) ? "PM" : "AM";

You definitely have a logic error in your am/pm calculation, but I don't think this ternary is the answer. Feel free to check me on that.

 

Couple of things that seem maybe off at a glance: The am/pm calculation as mentioned, your calculation for minutes might allow you to go to x:60 which I guess teeeeechnically isn't wrong(?) but probably not what you were looking for, and you'll want to pad out minutes < 10 otherwise you could end up with x:1 instead of x:01.

 

In any case I always found it helpful to walk through. Take us through a run of your code line by line. What does it do? What did you expect it to do? Set breakpoints if that helps. Hopefully in the process the problem will become clear, and if not we might be able to see more easily.

 

EDIT - Basically everything @wanderingfool2 said.

Edited November 8, 2019 by jslowik
Someone beat me to it.

[Bacon soup]

3 minutes ago, jslowik said:

You definitely have a logic error in your am/pm calculation, but I don't think this ternary is the answer. Feel free to check me on that.

 

OP hasnt given us the entire code or problem, so I have to make assumptions about the things I can't see. For example I dont know why they are using integers and not an easier time format, like Java's own date/time libraries. I assume they are publishing to a UI, but from what to where we dont know.

5 minutes ago, wanderingfool2 said:

.if the getHour is 25 it would set it to PM even though it should be AM.

then use (getHour()%24 > 12). Doing a loop instead of simple math only makes things more complex and buggy. A page full of loops and if statements is terrible to read. Its really a failure of schools pushing people into coding and neglecting math.

 

[wanderingfool2]

1 minute ago, Bacon soup said:

OP hasnt given us the entire code or problem, so I have to make assumptions about the things I can't see. For example I dont know why they are using integers and not an easier time format, like Java's own date/time libraries. I assume they are publishing to a UI, but from what to where we dont know.

 

It was mentioned it is homework, the easiest assumption why they don't use Java's date/time library is because they are learning and it is a "simple" object to implement.

 

7 minutes ago, Bacon soup said:

then use (getHour()%24 > 12). Doing a loop instead of simple math only makes things more complex and buggy. A page full of loops and if statements is terrible to read. Its really a failure of schools pushing people into coding and neglecting math.

 

I wouldn't necessarily say it is neglecting math, it is about learning to code, and learning about optimizations and additional code designs later.  My point of using pure maths for something like this was that it reducing the chance of learning to code, but also hides certain thought processes behind it.

 

While I am not condoning the while loop as a great solution to the problem, it is important to recognize it is a good learning tool and a good way to begin tackling some problems.

 

In this case, the while loop I think is better for the final comprehension of the problem (e.g. the hour while loop is more better for quick comprehension than minute += m;hour = (hour + h + minute/60)%24; minute = minute%60;)

[Bacon soup]

2 hours ago, wanderingfool2 said:

it is important to recognize it is a good learning tool and a good way to begin tackling some problems.

It is not a learning tool. It is not good for this problem. It is an operation. It has a condition and an instruction to jump to a prev instruction. Iv seen people do this at university and they always struggled because it is a bad approach 

[CreepyPro]

public void advance(int a){
    int m =a%60; //Minutes to add
    int h =a/60; //Hours to add
    minute = minute + m; //Add minutes to m
    while(minute>60) { //Divides minutes by 60 to get the amount of hours in case the input is bigger than 60
        minute = minute - 60;
        hour++;
    }   
    hour = hour + h; //Adds the amount of hours that the while loop form line 5 got
    while(hour>12) { //Expected to change AM to PM if neccesary and change the amount of hours if it happens to be bigger than 12.
        if(getAmPm()=="AM") {
            amPm = "PM";
        } else
            amPm = "AM";
        hour = hour - 12;
    }
}       
        

Here I try to explain better what everything is suppose to do. The clock class is in this link under Clock-advance.

@Bacon soup

@jslowik

[wanderingfool2]

22 hours ago, Bacon soup said:

It is not a learning tool. It is not good for this problem. It is an operation. It has a condition and an instruction to jump to a prev instruction. Iv seen people do this at university and they always struggled because it is a bad approach 

It depends what you are learning.  If we are talking about design choices that critically, aside from the mathematical way which would be like 5 lines of code, the way you suggested thing about incrementMinute and incrementHour creates more ambiguity/tighter coupling of non-mutually exclusive tasks (incrementMinute would effect the hour).  There are many university math level students who struggle with the concept of while/for loops, so an understanding of them can be a learning tool.  I am going to assume though you originally didn't really read the original code, given the oversights you made though in understanding what his code was doing.

 

@CreepyPro  If I may ask, what part is it currently failing at now?  In your recent bit you still have the line

while(minute>60) //<-- this is still likely the error when minute = 60 this loop doesn't get run.  60 is the boundary case. while(minute>=60)

[Bacon soup]

I would store time in one variable. Much easier.

private int time;

public Clock(int h, int m, String ap){
	setTime(h*60 + m);//this might look weird to put in a method. The reason I did this is, although we dont need the hour and minute variables, I think we have to set the hour and minute variables to pass the test.
    ...
}

public void setTime(int time_){
	time = time_;
    hour = getHour();
    minute = getMinute();
}
public int getMinute(){
	return time%60;
}
public int getHour(){
	return time/60;
{

public void advance(int a){
	setTime(time + a); //see how easy that was. I would do time += a but im predicting we have to set the hour and minute variables
    .....
    ....
    ....
    ....
}

"//Expected to change AM to PM if neccesary and change the amount of hours if it happens to be bigger than 12."

//use the remainder of division by 24 to determine am/pm
if(hour%24 >= 12) 
	amPm = "PM";
else 
	amPm = "AM";

//use this to reduce to 12-hour
	hour = hour % 12;
	if(hour == 0) hour = 12;

see its all simple math. No loops. Loops are not "learning tools" as someone suggested. No amount of programming loops can teach you math.

 

 

[Bacon soup]

I jsut noticed this

Quote

Assume that the state of the object is valid at the start of the call and that the amPm field stores either "AM" or "PM"

The way you assume the state of a string is to do this:

public static final String AM = "AM";
public static final String PM = "PM";

So where ever you use "AM" you can use AM and it remains consistent and predictable.

 

[wanderingfool2]

3 hours ago, Bacon soup said:

I would store time in one variable. Much easier.

see its all simple math. No loops. Loops are not "learning tools" as someone suggested. No amount of programming loops can teach you math.

 

 

 

I did not say it teaches you math.  I have seen math students struggle with while loops, so yes learning to do while loops properly even when there may be better methods can be a learning tool.  A restriction in tools you are able to use can create a learning experience (have you not taken courses where the instructor specifies not being allowed to use certain API calls and such; as it simplifies the problem too much or cuts out the portion of though processes you are suppose to learn).  A classical example is learning coding the fib sequence.  Usually they start with recursion (f(x) = f(x-1)+f(x-2)) code, then move to implementing into a loop.  (Because it teaches you recursion, not because recursion is a good way to solve the problem).  Strictly speaking you could implement the fib sequence in a single line (working for the larger numbers)...but that way is never taught in coding because it isn't really learning if all you do is type in a formula.

 

In a quick summary, these types of examples aren't meant to learn math (and I never claimed as much); but it is teaching other things which are apparently too subtle for you Bacon.  Actually on top of that he isn't allowed changing the implementation of the other methods.  You implementation is also wrong, and also introduces an y2k type of bug (compared to his implementation which could run indefinitely yours will break...albeit he needs to fix the minutes>60 issue, but the remaining logic appears to be correct.  Not optimized but his error creates less of an issue than your "simple" implementation that has a fault)

 

2 hours ago, Bacon soup said:

I jsut noticed this

The way you assume the state of a string is to do this:

public static final String AM = "AM";
public static final String PM = "PM";

So where ever you use "AM" you can use AM and it remains consistent and predictable.

 

How long has it been since you have actually taken/reviewed questions from school?  That line you quoted is merely saying that everything was initialized correctly and that the amPM is expected to be AM or PM...but that does not require the use of constants (yes I do agree that constants should be used whenever something is intended to be static as it prevents magic numbers within the code, but this is an homework assignment that specifically is specifically asking to add a method, and has a spot for it...adding in more variables and constants is just asking for point reduction, as the method may just be automatically grabbed, and parsed so those lovely variables added outside of the method may break the automated grading...if there is one).

[Bacon soup]

7 hours ago, wanderingfool2 said:

it is teaching other things which are apparently too subtle for you Bacon

please dont reply to me with a rude page long essay that only serves to satiate your ego. I'm not reading your literature. If using a sinlge variable is not suitable for homework then we can use the ones supplied without much work and without your rude language. My suggestion to OP is still: ditch the loops and correct the maths.

public void advance(int a){
	hour += a/60 //add hours
    if amPm.equals("PM") hour += 12 //Im assume hour is in 12 hour format so add 12 for after noon to work with code below
    
    hour = hour %24 //reduce it to 24 hours
	if(hour%24 >= 12) 
		amPm = "PM";
	else 
		amPm = "AM";
	//use this to reduce back to 12-hour
	hour = hour % 12;
	if(hour == 0) hour = 12;
    
    minute += a%60 //add minutes
    minute = minute%60 //reduce to under 60

}

@wanderingfool2 before you reply to me next, please keep in mind that i can be a fucking rude cunt too.

[wanderingfool2]

5 minutes ago, Bacon soup said:

please dont reply to me with a rude page long essay that only serves to satiate your ego. I'm not reading your literature. If using a sinlge variable is not suitable for homework then we can use the ones supplied without much work and without your rude language. My suggestion to OP is still: ditch the loops and correct the maths.

public void advance(int a){
	hour += a/60 //add hours
    if amPm.equals("PM") hour += 12 //Im assume hour is in 12 hour format so add 12 for after noon to work with code below
    
    hour = hour %24 //reduce it to 24 hours
	if(hour%24 >= 12) 
		amPm = "PM";
	else 
		amPm = "AM";
	//use this to reduce back to 12-hour
	hour = hour % 12;
	if(hour == 0) hour = 12;
    
    minute += a%60 //add minutes
    minute = minute%60 //reduce to under 60

}

@wanderingfool2 before you reply to me next, please keep in mind that i can be a fucking rude cunt too.

This has nothing to do with my ego, I am just going off the fact following:

 - that none of your posts has gotten the answer right (Yes, the code you posted above has an oversight in it again)

 - you incorrectly interpreted the original code, which btw is only missing an = sign in terms of getting the correct solution

 - the original post literally says explain why his code didn't work (jslowik and I both mentioned and you didn't).  He isn't looking for the way to do it, but rather why his code is failing.

 - you called me out about "learning tools" without the comprehension of what I was evening talking about (and given that you made mention of making assumptions like asking why they didn't use java's time library to me shows that you haven't put consideration into what was asked)

 

So I stick by my comment, especially since you keep going on about getting rid of the loops and correct maths and yet your code has yet another logic error in it. (i=1...600 { clock.advance(1)  } would fail)