Alarm clock with "ringing" functionality

Question

I have a question regarding a program that is to run an alarm clock and display the time (HH:MM AM/PM) and also trigger the alarm and display "ringing" at 12:00 AM.

I am a beginner in Java and I am a little lost. I believe I have most of the skeletal work done. This is much like another question asked on here but I could not find an answer to my problems. So I apologize for the similarity. I am also to create a "dummy driver" in order to test the method as one of the requirements for this program is to be able to be read by a general driver someone else has created. Therefore, all of my variables and methods must be left the way I have them in order to be read by said driver. I wish I could be a little more specific in what I need assistance with but any feedback you can give me is much appreciated.

This is what I've got so far:

//dummy driver
package program5;

public class Program5 {

public static void main(String[] args) {
AlarmClock alarm = new AlarmClock();
System.out.println(alarm.getHour() + ":" + alarm.getMinute() + alarm.getAmOrPM());
System.out.println(alarm.getAlarmHour() + ":" + alarm.getAlarmMinute() + alarm.getAlarmAmOrPm());
System.out.println(alarm.isIsAlarmRinging());
}
} 

package program5;

public class AlarmClock {

private int hour;
private int minute;
private String amOrPm;
private int alarmHour;
private int alarmMinute;
private String alarmAmOrPm;
private boolean isAlarmRinging;

//constructor:
AlarmClock() {
hour = 12;
alarmHour = 12;
minute = 0;
alarmMinute = 0;
alarmAmOrPm = new String();
amOrPm = new String();
isAlarmRinging = false;
} 

//getters:
public int getHour() {
if ((hour >= 1) && (hour <= 12)) {
}
return hour;
}

public int getMinute() {
if ((minute >= 0) && (minute <= 59)) {
}
return minute;
}

public String getAmOrPM() {
return amOrPm;
}

public int getAlarmHour() {
return alarmHour;
}

public int getAlarmMinute() {
return alarmMinute;
}

public String getAlarmAmOrPm() {
return alarmAmOrPm;
}

public boolean isIsAlarmRinging() {
if (hour >= 12){
isAlarmRinging = true;
}
return isAlarmRinging;
}

//setters
public void setHour(int newHour) {
if ((newHour >= 1) && (newHour <= 12)) {
    hour = newHour;
}
}

public void setMinute(int newMinute) {
if ((newMinute >= 0) && (newMinute <= 59)) {
    minute = newMinute;
}
}

public void setAmOrPM(String newAmOrPm) {
amOrPm = newAmOrPm;
}

public void setAlarmHour(int newAlarmHour) {
if ((newAlarmHour >= 1) && (newAlarmHour <= 12)) {
    alarmHour = newAlarmHour;
}
}

public void setAlarmMinute(int newAlarmMinute) {
if ((newAlarmMinute >= 0) && (newAlarmMinute <= 59)) {
    alarmMinute = newAlarmMinute;
}
}

public void setAlarmAmOrPm(String newAlarmAmOrPm) {
alarmAmOrPm = newAlarmAmOrPm;
}

public void setIsAlarmRinging(boolean isAlarmRinging) {
if (isAlarmRinging = true) {
    System.out.println("Ringing!");
}
}

public void advanceOneMinute() {
minute++;
if (minute == 59) {
    hour++;
}
}

public void advanceMinutes(int minutesToAdvance) {

}

public void advanceOneHour() {
hour++;
if (hour > 12) {
hour = 1;
}    
}

public void advanceHours(int hoursToAdvance) {

}

public void setTime(int newHour, int newMinute, String newAmOrPm) {
hour = newHour;
amOrPm = newAmOrPm;
}

public void setAlarmTime(int newAlarmHour, int newAlarmMinute, String newAlarmAmOrPm) {

if(hour >=0 && hour <= 23 && minute >=0 && minute <= 59) { 
newAlarmHour = hour; 
newAlarmMinute = minute;
} 
}

public void turnOffAlarm() {
isAlarmRinging = false;
}

public void displayTime() {

}

public void displayAlarmTime() {

}


}

It's far from done but it's a start. If you can tell me anything I can do, I'll be very thankful. I'm not sure exactly what my question is. Just if I'm on the right track and what I'm doing wrong. Sorry it's not a short, concise, clear-cut question.

Answer 1

Calendar

You created an alarmclock, which is really just a time-representation. A Calendar has both a date and time component. I strongly, strongly suggest you use that so you can let your user define how he wants his date represented and let the API handle fancy stuff like timezones, summer hours, etc.

You can also look into the Java 8 time API but I think that might not be appropriate yet.

Enums

When you have a limited set of different possibilities, your mind should immediately wander to enums. This way you can represent your AM/PM by saying DayPart.AM and DayPart.PM instead of the literal string "AM" which could mistakenly be used as "aM", "am", "AX", etc. It will provide compile-time safety and is a lot more pleasant to work with.

Indentation

Stick to conventions. Some people indent a block with one tab (or 4 spaces), some with 2 spaces. This will keep the code readable for everyone.

Methods without a meaningful body

Look at your code here:

public int getHour() {
if ((hour >= 1) && (hour <= 12)) {
}
return hour;
}

With formatting this becomes this:

public int getHour() {
    if ((hour >= 1) && (hour <= 12)) {
    }

    return hour;
}

Can you tell what's wrong here? Note that there are several methods that have this construct!

Assignment vs Comparison

This is a very common mistake:

if (isAlarmRinging = true)

What you do here is set isAlarmRinging to true, you never compare it.

Keep in mind:

• Comparison: ==.
• Assignment: =.

Unused parameters

In your method setTime you never use the parameter newMinute.
Likewise in method setAlarmTime you never use newAlarmAmOrPm.

Answer 2

One bug is that your constructor initializes the clock to an invalid state: amOrPm is set to an empty string.

You keep a seven state variables: hour, minute, amOrPm, alarmHour, alarmMinute, alarmAmOrPm, isAlarmRinging. Maintaining hour and minute separately, doing the carrying manually, is troublesome.

I suggest paring that down to three: time, alarmTime, isAlarmRinging. Keep time as the number of minutes since midnight.

public class AlarmClock {
    // Time, in minutes since midnight
    private int time;

    private static final int MINUTES_PER_12_HOURS = 12 * 60,
                             MINUTES_PER_DAY = 24 * 60;

    /**
     * Returns the hour, between 1 and 12 inclusive.
     */
    public int getHour() {
        int h = (this.time / 60) % 12;
        return (h == 0) ? 12 : h;
    }

    public int getMinute() {
        return this.time % 60;
    }

    public String getAmOrPm() {
        return (this.time < MINUTES_PER_12_HOURS) ? "AM" : "PM";
    }

    public void advanceOneHour() {
        this.time = (this.time + 60) % MINUTES_PER_DAY;
    }

    …
}

You don't even need an explicit constructor! The default values will cause the time to be initialized to midnight.

I'll leave it to you to fill in the rest.

Watch the consistency of your capitalization in getAmOrPM(). Either getAMOrPM() or getAmOrPm() would be acceptable; I prefer the latter, especially since it matches the way you named your amOrPm variable.

Answer 3

I'm not too familiar with Java, so I won't review any specific aspect of that. I just want to address the lack of indentation in this code, which is very important.

Let's take the first portion of your code:

public class Program5 {

public static void main(String[] args) {
AlarmClock alarm = new AlarmClock();
System.out.println(alarm.getHour() + ":" + alarm.getMinute() + alarm.getAmOrPM());
System.out.println(alarm.getAlarmHour() + ":" + alarm.getAlarmMinute() + alarm.getAlarmAmOrPm());
System.out.println(alarm.isIsAlarmRinging());
}
}

Whenever you have something inside of {} that are on multiple lines, the code within the {} should be indented so that readers will know that the code belongs in this block. In this code portion, it's hard to tell what belongs inside of what because everything is aligned towards the side.

In this portion, when you have a line that opens with a {, such as

public class Program5 {

the code within this containment should be indented. The number of spaces to indent by is not concrete and mostly depends on the environment and language. In this answer, I will use four spaces.

If we follow this idea of indentation, that code portion should look like this:

public class Program5 {

    public static void main(String[] args) {
        AlarmClock alarm = new AlarmClock();
        System.out.println(alarm.getHour() + ":" + alarm.getMinute() + alarm.getAmOrPM());
        System.out.println(alarm.getAlarmHour() + ":" + alarm.getAlarmMinute() + alarm.getAlarmAmOrPm());
        System.out.println(alarm.isIsAlarmRinging());
    }
}

There are two {, so the following code for each of those statements is indented. You can now see what belongs to what, thereby increasing readability.