Sign in to follow this  
banhminds

help me with this Timer Class I wrote

Recommended Posts

I'm in a good mood so if you would like to tell me how awful it is go ahead. I'm just now learning how to write OOP classes, and make them work. I'm writing a game right now, and I wanted a timer for the splash screen, if the user is too long on the menu, for the credits page, and for each level have a countdown timer, so I figured let's first look at the Java API and see if there isn't one already made. Well, there was in both the java.Util, and javax.swing so I tried to use it, but didn't get the results I wanted, so then I tried my best to write one myself, and here it is. Please feel free to give me any suggestions, comments, or anything else. It's going in a game. We run both the Timer() and CountDown() in the run() method of the main JApplet, Applet, JFrame, or Frame whichever your making it with, and we're guessing that it hits the method every 10000 milliseonds not sure. But, cretique please I want this to be my first really good class I can add to almost any java project I make. Needs a little help, and I'm hoping the little help it needs I'll get here. Thanks,
[sourcecode lang="java"]
/**             My Timer, StopWatch Class I've been working on that I so want to be done
 * Programmer:  c.s. Finch
 * Date:        05-25-07
 */

import java.awt.*;
import java.awt.event.*;
import java.awt.image.*;

public class Time 
{
    public int mili_secs, secs, mins, hrs;
    public int mili_counter, secs_counter, mins_counter, hrs_counter;
    String daTime;
    Font myFont = new Font("Comis Sans MS", Font.BOLD, 27);
    private int t_Flag;
    public boolean setTime;
    
    public Time(int t_Flag)
    {
        setTime = true;
        this.t_Flag  = t_Flag;
        
        mili_counter = 0;
        secs_counter = 0;
        mins_counter = 0;
        hrs_counter  = 0;
        
    }
    
    
    
    public void setTimer(int hrs, int mins, int secs, int mili_secs)
    {
        if(setTime)
        {
             this.mili_secs = mili_secs;
             this.secs = secs;
             this.mins = mins;
             this.hrs = hrs;
        }
    }
    
    public void addTime(int hours, int minutes, int seconds, int mili_seconds)
    {
        hrs += hours;
        mins += minutes;
        secs += seconds;
        mili_secs += mili_seconds;
    }
    
    public int setMiliSeconds(int mili_seconds)
    {
        mili_secs = mili_seconds;
        return mili_secs;
    }
    
    public int setSeconds(int seconds)
    {
        secs = seconds;
        return secs;
    }
    
    public int setMinutes(int minutes)
    {
        mins = minutes;
        return mins;
    }
    
    public int setHours(int hours)
    {
        hrs = hours;
        return hrs;
    }
    
    public void Timer()
    {
        
         
          mili_secs ++;
            
          if(mili_secs == 15000)
          {
              secs += 1;
              mili_secs = 0;
          }
            
          if(secs == 60)
          {
             mins += 1;
             secs = 0;
          }
            
          if(mins == 60)
          {
             hrs += 1;
             mins = 0;
          }     
        
        daTime = String.valueOf(hrs) + ":" + String.valueOf(mins) + ":" + String.valueOf(secs);
        
    }
    
    public void CountDown()
    {
        mili_secs --;
                
        if(mili_secs == -15000)
        {
            secs -= 1;
            mili_secs = 0;
        }
                
        if(secs == 0)
        {
            mins -= 1;
            secs = 59;
        }
                
        if(mins == 0)
        {
           hrs -= 1;
           mins = 59;
        }
         
         if((hrs <= -1) || (mins <= -1) || (secs <= -1)) daTime = "Times Up!";
         else daTime = String.valueOf(hrs) + ":" + String.valueOf(mins) + ":" + String.valueOf(secs);
    }
    
    public void paintTime(Graphics g, int x1, int y1)
    {
        g.setFont(myFont);
        g.setColor(Color.white);
        g.drawString(daTime, x1, y1);
        
    }
}
[\sourcecode]

Share this post


Link to post
Share on other sites

First, the name "Time" is a very vague name that doesn't tell us what the purpose of the class actually is. Something like GraphicalTimer would be a better name.

Your actual time handling is very brittle and you're doing no error checking. And I have no idea where the -15000 is coming from. You can simplify everything vastly by storing exactly one value in your class: the number of milliseconds left. Then your constructor will need to convert the hours/minutes/seconds to milliseconds, and your paintTime method will need to convert the number of milliseconds back to hour/minutes/seconds, but the actual counting down will be very simple.

I'd replace the setHours, setMinutes etc. method by a single "setTime(hours, minutes, seconds, millis)" method. I can't see anyone wanting to just set one component of the time.

Your CountDown method decrements the number of milliseconds by one. That means you have to call it 1000 times a second to be accurate. This is a waste because you're probably not going to update the graphics 1000 times a second. Instead, I'd pass a parameter to CountDown that indicates how much time in milliseconds has passed.

The message "Times up!" is hardcoded in your class. You might want to give the user the option of setting a different message. You will also need a method for checking if the time is up without displaying anything (e.g. for triggering Game Over if the player goes over the time limit).

Timer() is a really bad method name. First, it's a convention in Java to not start method names with capital letters. Second, Timer is a noun. I'd rename it to "countUp", and introduce a new parameter like in CountDown (which should be countDown).





Share this post


Link to post
Share on other sites
Any good class must start not with its implementation, but with its responsibility. What is essential is how the class can and should be used, with the implementation being a collateral detail of its usage.

Your class violates a fundamental principle in Object-Oriented Programming: the Single Responsibility Principle. The idea behind this is that a class should only be used to do one thing, should only be usable to do one thing, and should do that thing well. If you need to do something else, you should use another class, possibly in conjunction with the first.

Your class:
  1. Is responsible for advancing the timer based on real-world time.
  2. Is responsible for handling the hour, minute, second, millisecond representation of time, and conversion to a string.
  3. Is responsible for displaying the time.


This, in itself, means that you should cut your existing code into three different classes. Some of these classes may or may not already exist in the Java library. In particular, java.util.Date handles responsibility 2 above.


import java.util.Date;

/** Handles counting down from an initial time value. Handles
* pausing and altering remaining time.
* @author Victor Nicollet
* @version 1.0
*/

public class CountDown
{
/** Construct a countdown with a given time remaining.
* @param remaining Remaining time, in milliseconds.
* @param paused Is the countdown initially paused?
*/

public CountDown(int remaining, boolean paused)
{
this.startTime = (new Date()).getTime();
this.pauseTime = this.startTime;
this.fullTime = remaining;
this.isPaused = paused;
}

/** Remove the pause state. The countdown starts counting
* again.
* @see pause
*/

public void resume()
{
if (! this.isPaused) return;

int now = (new Date()).getTime();
int delta = now - this.pauseTime;
this.startTime += delta;

this.isPaused = false;
}

/** Enable the pause state. The countdown stops counting
* until it is resumed.
* @see resume
*/

public void pause()
{
if (this.isPaused) return;

this.isPaused = true;
this.pauseTime = (new Date()).getTime();
}

/** Get the remaining time, or zero if the countdown has
* ended.
* @return The remaining time, in milliseconds.
*/

public int remaining()
{
int elapsed;

if (this.isPaused)
elapsed = this.pauseTime - this.startTime;
else
elapsed = (new Date()).getTime() - this.startTime;

if (elapsed > this.fullTime)
return 0;
else
return this.fullTime - elapsed;
}

/** Determine if the countdown has ended.
* @return True if and only if the time is up.
*/

public bool isOver()
{
return (this.remaining() == 0);
}

/** Add or substract time to the countdown. May not
* decrease the remaining time below zero (value is
* capped otherwise).
* @param time The amount of time added, in milliseconds.
*/

public void addTime(int time)
{
int remaining = this.remaining();

if (remaining + time < 0)
time = - remaining;

this.fullTime += time;
}

/** Determines if the countdown is currently paused.
* @return True if and only if the countdown is paused.
*/

public boolean paused()
{
return this.isPaused;
}

// --- Private implementation --------------------------------

// The time at which the countdown started, in milliseconds
private int startTime;

// The time at which the current pause was enabled, if any,
// in milliseconds
private int pauseTime;

// The full countdown time, in milliseconds
private int fullTime;

// Whether the object is currently paused
private boolean isPaused;
};


Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this