Positional change program in Java

Question

I've written a program and it needs a review by skilled programmers.

The programs goal is to change 0-s to X and X-s to 0. The rule is X or O can move only to an empty place and x or o can only "jump" over one X or O. Program looks like:

------------------
|x|x|x|x| |o|o|o|o|
------------------

User input

move from? (0-8)

3

move to? (0-8)

4

Output

 -----------------
|x|x|x| |x|o|o|o|o|
------------------

package tornasz;
import java.util.Scanner;

public class Tornasz {


    public static void main(String[] args) {
        Scanner sc = new Scanner(System.in);
        char[] table = {'x', 'x', 'x', 'x', ' ', 'o', 'o', 'o', 'o'};
        int [] move = new int[2];
        int result;
        int counter = 0;

        do
        {
            drawPlayfield(table, counter);
            move = getValidMove(sc,table,move);
            makeMove(table,move);
            result = checkWin(table);
            counter ++;
        }
        while (result == 0);
        drawPlayfield(table,counter);
        displayWinner(result);
    }

    public static void drawPlayfield(char [] table, int counter){
        System.out.println("Lépésszám: "+counter);
        System.out.println("------------------");
        System.out.println("|"+table[0]+"|"+table[1]+"|"+table[2]+
                "|"+table[3]+"|"+table[4]+"|"+table[5]+"|"+table[6]+"|"+table[7]
        +"|"+table[8]+"|");
        System.out.println("------------------");
        counter++;
    }
    public static int [] getValidMove(Scanner sc, char [] table, int [] move)
    {
        System.out.println("Melyik mezővel lépsz? (0-8)");
        int start = sc.nextInt();
        if (start == -1)
        {
            System.exit(0);
        }
        System.out.println("Melyik mezőre lépsz? (0-8)");
        int destination = sc.nextInt();
        if (destination == -1)
        {
            System.exit(0);
        }
        while (start < 0 || start > 8 || destination < 0 || destination > 8)
        {
            System.out.println("Érvénytelen lépés!");
            System.out.println("Melyik mezővel lépsz? (0-8)");
            start = sc.nextInt();
            if (start == -1)
            {
                System.exit(0);
            }
            System.out.println("Melyik mezőre lépsz? (0-8)");
            destination = sc.nextInt();
            if (destination == -1)
            {
                System.exit(0);
            }
        }
        while (table[destination] != ' ')
        {
            System.out.println("Érvénytelen lépés!");
            System.out.println("Melyik mezővel lépsz? (0-8)");
            start = sc.nextInt();
            System.out.println("Melyik mezőre lépsz? (0-8)");
            destination = sc.nextInt();
        }

        int [] movement = new int[2];
        movement[0] = start;
        movement[1] = destination;
        return movement;
    }
    public static void makeMove(char [] table, int [] move)
    {
        table[move[1]] = table[move[0]];
        table[move[0]] = ' '; 
    }
    public static int checkWin(char [] table)
    {
        if (table[0] == 'o' && table[1] == 'o' && table[2] == 'o' &&
                table[3] == 'o' && table[4] == ' ' && table[5] == 'x' &&
                table[6] == 'x' && table[7] == 'x' && table[8] == 'x')
        {
        return 1;
        }
        return 0;
    }
    public static void displayWinner(int result)
    {
        if (result == 1)
        {
            System.out.println("Gratulálok, vége a játéknak!");
        }
    }

}

Now I only need to set the movement rules. I think it will be an another method with boolean.

Welcome to Code Review! Code Review is a platform where you can get feedback on working code to make it better. As you say, your code does not work as intended. Please read the how to ask section in the Help Center. — AlexV
– AlexV, Commented Mar 26, 2019 at 9:23
I changed the table from String to char because strings comparison is not the == operator. — Falcon83
– Falcon83, Commented Mar 26, 2019 at 9:31
As far as I can understand your description, each piece has always zero or one possible move, so you needn't ask both questions. However, if you ask the first question only (which piece to move, but not where it has to land), you'd have to devise the appropriate move yourself. — CiaPan
– CiaPan, Commented Mar 26, 2019 at 10:07
The code is incomplete and doesn't look like running and working correctly – the makeMove routine can only place "x" and not "o" into the table, anyway it's never invoked. I'm deleting my answer and voting to migrate the question to StackOverflow. — CiaPan
– CiaPan, Commented Mar 26, 2019 at 10:37
The code seems improved now with all substantial errors fixed. IMHO the question deserves to be reopened. — CiaPan
– CiaPan, Commented Mar 27, 2019 at 15:46

CiaPan · Accepted Answer · 2019-03-27 14:51:33Z

~~The move variable seems unnecessary - you pass it to getValidMove() but never use it inside the function.~~
The language has loops for repetitive tasks. In the drawPlayfield() you may do
```
for(int i=0; i<9; i++)
    System.out.println("|"+table[i]);
System.out.println("|");
```
instead of
```
System.out.println("|"+table[0]+"|"+table[1]+"|"+table[2]+
        "|"+table[3]+"|"+table[4]+"|"+table[5]+"|"+table[6]+"|"+table[7]
+"|"+table[8]+"|");
```
This costs 10 calls instead of two, but 1) saves strings addition, 2) makes the code easy expandable (you just need to replace the 9 with another expression).
~~The getValidMove() function returns just one int value. Why do you allocate a table for it? declare the return type as int instead of int [] and save unnecessary work.~~
There is some problem with reponsibilities of functions. Function getValidMove() not only gets a valid move, as its name says, but it also removes the chosen piece from the table with assignment table[start] = " "; That is a part of performing the actual move. However I can't see where the move is completed. Where do you do table[destination] = "x"; or table[destination] = "o";? And where do you store the shape of the piece removed by table[start] = " "; to be reinserted at destination?

Critical problems are fixed now - but one important remains:

The part to input user's move is unsafe. The first loop tests for both numbers being in the required range and repeats asking for input until data are correct – or until -1 appears, which terminates the program.

Then, however, another loop starts, which tests if the chosen destination position is free – and this loop does no longer validate values for being between 1 and 8. It also does not validate data for conforming the game rules (i.e. whether the jump is no more than 2 positions long).

IMHO there should be one loop, looking more or less like this:

do
{
    do
    {
        System.out.println("Input start position (0-8)");
        start = sc.nextInt();
        if (start == -1)
        {
            System.exit(0);
        }
    } while (start < 0 || start > 8);

    do
    {
        System.out.println("Input destination position (0-8)");
        destination = sc.nextInt();
        if (destination == -1)
        {
            System.exit(0);
        }
    } while (destination < 0 || destination > 8);

    if (destination == start)
    {
        System.out.println("Destination must differ from start!");
        continue;
    }
    if (destination < start - 2 || destination > start + 2)
    {
        System.out.println("Destination must not differ from start more than by 2!");
        continue;
    }

    if (table[destination] != ' ')
    {
        System.out.println("The destination position is not empty!");
        continue;
    }
} while (false);   // the input is OK now

Of course the two inner loops are asking for replacing them with a call to some helper function...

The drawPlayfield() method is a requirement in the task thats why I used it and not making a for loop for the task. — Falcon83
– Falcon83, Commented Apr 3, 2019 at 17:26
@Falcon83 AFAIK I did not suggest you shouldn't use drawPlayfield(). I just proposed to write it in another way, which IMO is a bit more readable and more flexible. — CiaPan
– CiaPan, Commented Apr 3, 2019 at 17:36

Stack Exchange Network

Positional change program in Java

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Positional change program in Java

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions