Programming a simple clock

Question

Here's the start of some code for a clock that can display time in 24 and 12 hours.

What could be added to it? How could I can apply notions such as OOP, model and Enums here?

public Clock {
 private int time = 0;
 public int get24hrTime(){
 return time;
 }
 public int set24hrTime( int newTime ){
 this.time = newTime;
 }
 public int get12hrTime(){
 if( time - 12 < 0 ){
 return time;
 } else {
 return time - 12;
 }
 }
 public String get12hrPostfix(){
 if( hours - 12 < 0 ){
 return "AM";
 } else {
 return "PM";
 }
 }
}

• Adam Siemion: "Do not fix something that works", what is the issue with this code?
• James P.: There is no functional issue with this code. It will get the green flag in TDD. But I am bothered by the design aspect. I just want to see how other programmers could design this more elegantly with comments on their reasoning.

Answer 1

Why do you check the time with (hours - 12 < 0). This involves two operations, one subtraction and one compare. You could simply use (hours < 12).

Also you rely that in the method set24hrTime(int newTime) the time is never> 23 for your code to work. Why don't you check this?

Also what about the value 0 for your hours? This must be displayed as 12 AM. At the moment you return 0 in this case where it must be 12.

Also for the value 12 of the hours you return 0 as value for the 12hrTime which is wrong.

To sum up I would suggest the following code:

public Clock {
 private int time = 0;
 public int get24hrTime() {
 return time;
 }
 public int set24hrTime(int newTime) {
 if (newTime > 23 || newTime < 0) {
 throw new IllegalArgumentException();
 }
 this.time = newTime;
 }
 public int get12hrTime(){
 if (time == 0) {
 return 12; // Special case 0
 } else if (time <= 12) {
 return time;
 } else {
 return time - 12;
 }
 }
 public String get12hrPostfix() {
 if (time < 12) {
 return "AM";
 } else {
 return "PM";
 }
 }
}

• Connor: Where does the "throw new IlllegalArguemntExeption()" come from and what does it do?
• Uwe Plonus: The IllegalArgumentException is a RuntimeException of Java. You can throw this exception if you detect an illegal argument (e.g. a negative hour or an hour above 23).