Object oriented programming - question about design and accessing relevant data

Question

Consider you have the following code in a Timer: It's main goal is to count down the seconds and to show it on GUI, which is Swing based. The Timer is part of the game and is used for a decision of who is the winner by taking the user who reached a solution first.

Action updateClockAction = new AbstractAction() {

    public void actionPerformed(ActionEvent e) {

        JLabel secLabel = m_GameApplet.GetJpanelStartNetGame().GetJlabelSeconds();
        secLabel.setFont(new java.awt.Font("Lucida Handwriting", 1, 36));
        secLabel.setForeground(Color.red);
        secLabel.setText(Integer.toString(m_TimerSeconds));
        if (m_TimerSeconds > 0) {
            m_TimerSeconds--;
        } else if (m_TimerSeconds == 0) {
            m_Timer.stop();
            m_GameApplet.GetJpanelStartNetGame().GetJlabelSeconds().setText("0");
            m_GameApplet.GetJpanelStartNetGame().GetJbuttonFinish().setVisible(false);
            //Checking whether time ended for both players and no solution was recieved
            if (!m_WasGameDecisived) {
                System.out.println("Tie - No one had a solution in the given time");
            }
        }
    }
};
m_Timer  = new Timer(1000, updateClockAction);

Now, I have two main packages in the client implementation A.GUI - hold all the swing Jpanels etc. B.LogicEngine

Now in LogicEngine I have a class that is called GameManager- that should manage and store information about the game.

My Current implementation in general is like this: I have a JpanelMainGame which is on the GUI Package JpanelMainGame contains JPanelGameBoard which has reference(or in other word contains) an instance of GameManger, which is on different package - The LogicEngine package.

So my questions are these:

1. Where does all of the timer definition code above should be placed?

   A. In the JpanelMainGame? (JpanelMainGame should have a reference to it).

   B. In GameManger as a part of a data of the game

2. In each solution you give, How should I access all the label info from within the anonymous inner class? As I can only access member from the outer class with command: OuterClass.this.member.

3. Any comments about current design.

Thank you very much

Answer 1

I repeat the answer I gave on another question:

I strongly discourage from organizing packages from an implementational point of view, like controllers, data, etc. I prefer grouping them by functionality, that is, feature1, feature2, etc. If a feature is reasonably complex and requires a large number of classes, then (and only then) I create subpackages like above, that is, feature1.controllers, feature1.data, etc.

Answer 2

Well not to answer questions with questions, but...

• How often is Timer used?
• Is Timer coded to be general use, or specific to a certain class?

If the Timer is general use, then I would say it belongs in the LogicEngine package tree somewhere. However, if Timer can only be used in a GUI element, then it belongs in the GUI package tree.

As a general rule of thumb (not to bang on the Agile drum too hard), you should only code what makes sense now but not be afraid to change it later.

Example: You are making an anonymous inner class of the type AbstractAction() in your code. This is just fine (although I shy away from anon classes) if the updateClockAction variable is used simply. But once your coding leads you to the need to cross the boundries of updateClockAction (via OuterClass.this.member) then I assert its time for a bit of refactoring: extract this inner class and make it a fully qualified class. This way, you can have the proper amount of control over the updateClockAction object's internal state (using the cstr, getters, etc)

EDIT: Added requested example

public class Testing {
    private Timer timer; /* INIT this from somewhere.... */

    public void myFunction() {
    /* 60 Seconds */
    long countDownTimeSEC = 60; 
    /* convert to Miliseconds */
    long countDownTimeMS = 1000 * countDownTimeSEC; 

    /* Get ref to label */
    JLabel label = m_GameApplet.GetJpanelStartNetGame().GetJlabelSeconds(); 

    /* Set once */
    label.setFont(new java.awt.Font("Lucida Handwriting", 1, 36)); 
    label.setForeground(Color.red);

    /* Set initial time */
    label.setText(Long.toString(countDownTimeSEC)); 

    /* Get ref to button */
    JButton button = m_GameApplet.GetJpanelStartNetGame().GetJbuttonFinish(); 

    /* Set up post Count Down list */
    ArrayList<AbstractAction> actsWhenComplete = new ArrayList<AbstractAction>();

    /* instantiate countdown object */
    CountDownClockAction cdca = new CountDownClockAction(label, countDownTimeMS, actsWhenComplete);
    this.timer  = new Timer(1000, cdca);

    /* Now that we have a timer, add the post Count Down action(s) to the  post Count Down list */
    actsWhenComplete.add(new CountDownFinishAction(label, button, this.timer));


    /* Finally, kick off the timer */
    this.timer.start();
}

public static class CountDownClockAction extends AbstractAction {
    private static final long serialVersionUID = 1L;
    private final JLabel labelToUpdate;
    private final long startMS;
    private long currentMS;
    private long timeMark;
    private final ArrayList<AbstractAction> actionsToExeWhenComplete;
    private boolean actionsExedFlag;

    public CountDownClockAction(
            final JLabel labelToUpdate, 
            final long startMS, 
            ArrayList<AbstractAction> actionsToExeWhenComplete
    ) {
        super();
        this.labelToUpdate = labelToUpdate;
        this.startMS = startMS;
        this.currentMS = startMS;
        this.timeMark = 0;
        this.actionsExedFlag = false;
        this.actionsToExeWhenComplete = actionsToExeWhenComplete;
    }

    @Override
    public void actionPerformed(final ActionEvent e) {
        /* First time firing */
        if (this.timeMark == 0) 
            this.timeMark = System.currentTimeMillis();

        /* Although the Timer object was set to 1000ms intervals, 
         * the UpdateClockAction doesn't know this, nor should it
         * since > or < 1000ms intervals could happen to do the fact that
         * the JVM nor the OS have perfectly accurate timing, nor do they
         * have instantaneous code execution properties.
         * So, we should see what the *real* time diff is...
         */
        long timeDelta = System.currentTimeMillis() - this.timeMark;

        /* Allow for the label to be null */
        if (this.labelToUpdate != null)
            labelToUpdate.setText(Long.toString((long)(currentMS / 1000)));

        if (currentMS > 0) {
            currentMS -= timeDelta;

        } else if (currentMS <= 0 && this.actionsExedFlag == false) {
            /* Ensure actions only fired once */
            this.actionsExedFlag = true;
            /* Allow for the label to be null */            
            if (this.actionsToExeWhenComplete != null)
                for (AbstractAction aa: this.actionsToExeWhenComplete)
                aa.actionPerformed(e);
        }

        /* Finally, update timeMark for next calls */
        this.timeMark = System.currentTimeMillis();
    }
}

public static class CountDownFinishAction extends AbstractAction {
    private final JLabel labelToUpdate;
    private final JButton buttonToUpdate;
    private final Timer timerToStop;

    public CountDownFinishAction(
            JLabel labelToUpdate,
            JButton buttonToUpdate, 
            Timer timerToStop
    ) {
        super();
        this.labelToUpdate = labelToUpdate;
        this.buttonToUpdate = buttonToUpdate;
        this.timerToStop = timerToStop;
    }

    @Override
    public void actionPerformed(final ActionEvent e) {
        /* Perform actions, allowing for items to be null */
        if (this.labelToUpdate != null)
            this.labelToUpdate.setText("0");

        if (this.buttonToUpdate != null)
                this.buttonToUpdate.setVisible(false);

            if (this.timerToStop != null)
                this.timerToStop.stop();
        }
    }
}