How to check for full column?

Question

I am building a Connect Four game, and here's my checker:

 static final int ROWS = 6;

    public void checkIfFull(int colu)
{
    /*deleted*/
}

This is invoked when the user clicks on a button:

if (e.getSource () == b1)
    {
        checkIfFull (0);


    }

This is my method in positioning the token at the lowest open level:

    public void setPos(int column)
{
    if (demise) return; //this is for when the game is already over and the user
still attempts to click on a button

    int row;
    for (row = 0; row < ROWS; ++row)
    {
        if (slots[row][column] > 0)
            break;
    }

    if (row > 0)
    {
        //save current player
        slots[--row][column] = active;
        //change turns
        /*if (active == GREEN)
        {
            active = RED;
            activeMe ();
        }
        else if (active == RED)
        {
            active = GREEN;
            activeMe();
        } I moved this to setCircle method*/

        setCircle(active, row, column); //gui method used to fill up the empty circle
    }


}

As of the moment when I fill up a column, the Joptionpane doesn't pop up, but it's supposed to.

Any clues?

I'm still continuously studying the logic I have, so if there are other flaws you can see, please do point them out. :)

Stackoverflow is more for specific questions. What is the issue you're encountering? Are you receiving errors (if so, post the stack trace)? Are you observing unexpected behaviour, and if so, what is it, and what is the desired behaviour? — Paul Richter, Feb 09 '14 at 02:30
@Teeg Sorry, should've been more specific. at the moment the joptionpane is supposed to pop up when the column is full, but it doesn't — user3026693, Feb 09 '14 at 02:34
Great, thanks. This is not the cause of your error, but I can state with certainty, that your `public static void setPos(int column)` should not be declared *static*. If you do this to prevent a compilation error, then you're fixing the wrong thing. — Hovercraft Full Of Eels, Feb 09 '14 at 02:36
Also, you should not create a new JFrame for your JOptionPane but should use a component from the current displayed GUI for that. — Hovercraft Full Of Eels, Feb 09 '14 at 02:37
@HovercraftFullOfEels actually my class already directly extends JFrame, so in my code I actually set `JOptionPane.showMessageDialog(null)`, since I can't find anything that tells me how to reference to the frame that includes it — user3026693, Feb 09 '14 at 02:46
No, definitely don't use null. If this code is *in* the JFrame derived class, then either use `this`, or if the JOptionPane is displayed within an *inner* class, then use `OuterClassName.this`. And again, get rid of the static modifier. — Hovercraft Full Of Eels, Feb 09 '14 at 02:48
@HovercraftFullOfEels thank you, been great help. Mind me asking why the method should not be static? — user3026693, Feb 09 '14 at 02:55
If it is static, then it breaks object-oriented rules, and your class loses ability to inherit the method or maintain an object-dependent state. You should only use static methods for utility methods (such as `Math.abs(...)` and similar methods), which this is definitely not. — Hovercraft Full Of Eels, Feb 09 '14 at 03:17

score 1 · Accepted Answer · edited May 23 '17 at 10:25

1

You should check whether counter is equal to 6, since you have declared:

static final int ROWS = 6;

so you have to check this:

if (counter == ROWS) // not 5

Note that you are comparing against ROWS and not directly with 6 to avoid "magic numbers".

Also, don't forget to declare and initialize counter before using it:

int counter = 0;

edited May 23 '17 at 10:25

Community

1
1

answered Feb 09 '14 at 02:32

Christian Tapia

33,620
7
56
73

makes sense, but the loop starts at 0 though? so 5 ends up to be the max value? – user3026693 Feb 09 '14 at 02:35
Yes, but from 0 to 5, there are 6 numbers. – Christian Tapia Feb 09 '14 at 02:42
The condition in my loop though is `row < ROWS` so that works like 5 < 6 and then the loop breaks when it updates and row = 6? – user3026693 Feb 09 '14 at 02:47
The last condition checked will be: `6 < 6`, which is `false`, so the loop finishes. – Christian Tapia Feb 09 '14 at 02:49
Okay, I tried that, thanks. But now JOptionPane already pops up even though only 3 spaces have been filled – user3026693 Feb 09 '14 at 02:53
And by the way i initialized counter to 0, static – user3026693 Feb 09 '14 at 02:58
1

Why static? If you are just going to use `counter` to this specific task, make it a local variable. – Christian Tapia Feb 09 '14 at 03:00
If the user chooses to end the game the program will have to reset everything (scores and the elements of the array) to 0 from another method so I thought to declare it outside of any method and be accessible to the whole class – user3026693 Feb 09 '14 at 03:04
It really depends on the way you are implementing the game. But the solution I stated should work. Just don't forget to set `counter` to 0 before checking if the column is full. – Christian Tapia Feb 09 '14 at 03:07
It's already working now though even though I didn't really change anything else aside from the condition, like suddenly it just worked, leaving me confused but okay. Thanks – user3026693 Feb 09 '14 at 03:13
@user3026693: indeed, **why static**? Get rid of most **all** statics except for the main method and constants. – Hovercraft Full Of Eels Feb 09 '14 at 03:21
@HovercraftFullOfEels Yeah as I have mentioned I declared `counter` static so it can be accessible to the whole class and I don't have to update it like "method per method" (because I do use it for like at least 3 methods) if you get what I'm saying but that was probably too vague. And okay I will, thank you :) – user3026693 Feb 09 '14 at 04:00
@user3026693: that's not a good reason to make it static, seriously. – Hovercraft Full Of Eels Feb 09 '14 at 04:02
@HovercraftFullOfEels okay, i don't anymore. just the constants. I'm still trying to find my way around Java. thanks for pointing the static stuff out. – user3026693 Feb 09 '14 at 05:27

How to check for full column?

1 Answers1