CODEWITHSUNDEEP
javatimer
cutrightjm
cutrightjm

Reputation: 657

How am I messing up my Java timer?

This Morning Class is supposed to create a new Morning, and at the beginning of each a rooster will crow. My rooster will crow once, wait five seconds, and crow again.. Then 5 seconds later it crows twice, and after that it crows non-stop. What could I be doing that makes it do that? I just want it to crow every 5 seconds. If I put a timer.restart() in ActionPerformed, it does nothing. Could someone please point out or give me a tip to what I'm doing wrong? Any help would be much appreciated.

import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
import javax.swing.Timer;

public class Morning extends JFrame
    implements ActionListener
{
  private EasySound rooster;
  public Timer timer;

  public Morning()
  {
    super("Morning");

    EasySound rooster = new EasySound("roost.wav");
    rooster.play();

    timer = new Timer(5000, this);
    timer.start();

    Container c = getContentPane();
    c.setBackground(Color.WHITE);
    //page 35
  }

  public void actionPerformed(ActionEvent e)
  {
    Morning morning = new Morning();
  }


  public static void main(String[] args)
  {
    Morning morning = new Morning();
    morning.setSize(300, 150);
    morning.setDefaultCloseOperation(EXIT_ON_CLOSE);
    morning.setVisible(true);
  }
}

Upvotes: 0

Views: 494

Answers (3)

Hovercraft Full Of Eels
Hovercraft Full Of Eels

Reputation: 285415

You don't have an infinite loop but rather infinite recursion (albeit on the slow side). Don't recurse here -- don't create a new Morning in your Timer's actionPerformed. Instead just put in the method to make the crow sound heard.

Next time, please put your code in your post itself (as I've done for you). Don't make us go to other sites since if you're asking for free help, you want to make it as easy as possible for others to help you.

Edit
You're also shadowing the rooster variable declaring it twice, once int he class which is null and once in the constructor which is not null. Don't redeclare it so you initialize the class field not a local field.

Change this:

  public Morning()
  {
    super("Morning");

    EasySound rooster = new EasySound("roost.wav"); // this creates a *local* variable only
    rooster.play();

to this:

  public Morning()
  {
    super("Morning");

    // EasySound rooster = new EasySound("roost.wav");
    rooster = new EasySound("roost.wav");
    rooster.play();

Upvotes: 2

david a.
david a.

Reputation: 5291

Instead of instantiating the Morning class each time the timer fires, put the rooster.play() to it (also don't forget to make rooster a class field instead of a local variable). In the constructor, also make sure to call 'timer.setRepeats(true)'

Upvotes: 0

John B
John B

Reputation: 32969

Looks to me like you have an infinite loop. You create a new Morning, which starts the timer. When then timer goes off you create a new Morning, which starts the time, etc, etc, etc. Seems like you should be using just a single Morning object that keeps track of how many times it has crowed.

I would suggest setting the Timer to repeat, in actionPerformed crow the appropriate number of times based on how many times it has crowed, and stop the timer when you are crowing forever. This however, assumes that you want exactly 5 seconds between the start of the crows. If you need 5 seconds between the end of a crow and the start of the next, do not repeat but restart the time after you are done crowing.

Upvotes: 1

Related Questions