I am new to programming and wanted to know how this code looked.

  • (2 Pages)
  • +
  • 1
  • 2

19 Replies - 1395 Views - Last Post: 12 February 2017 - 09:59 PM Rate Topic: -----

#1 Rakin23   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 13
  • Joined: 01-February 17

I am new to programming and wanted to know how this code looked.

Posted 01 February 2017 - 06:12 AM

I am not getting any errors i just wanted input on ways to improve it.

Game class.
package com.company;

import java.util.ArrayList;
import java.util.Scanner;
import java.util.function.BiConsumer;

public class Game implements Runnable {

    private Scanner at = new Scanner(System.in);
    private boolean play;
    private static ArrayList<Integer> pile;
    static Player player1;
    static Player player2;


    public Game(){
        pile = new ArrayList<>(3);
        play = true;
    }

    @Override
    public String toString() {
        return "Pile A:" + pile.get(0) + "|Pile B:" + pile.get(1) + "|Pile C:" + pile.get(2);
    }


    public static ArrayList<Integer> getPile() {
        return pile;
    }
    private void input(){
        System.out.println("How many sticks to put in pile A, B and C");
        System.out.println("Pile A");
        int in = at.nextInt();
        pile.add(in);
        System.out.println("Pile B");
        in = at.nextInt();
        pile.add(in);
        System.out.println("Pile C");
        in = at.nextInt();
        pile.add(in);
        System.out.println(toString());
    }
    private void play(){
        while (Player.isTurn()) {
            Player.turn();
            if (!Player.isTurn()) {
                System.out.println(player2.getName() + " Has Won!!!");
                break;
            } else {
                player1.pick();
                Player.turn();
                if (!Player.isTurn()) {
                    System.out.println(player1.getName() + " Has Won!!!");
                    break;
                } else {
                    player2.pick();
                }
            }
        }
    }
    private void loop(){
        System.out.println("Do you want to play again?");
        if (at.next().equals("Yes")) {
            play = true;
        } else {
            play = false;
        }
        pile.removeAll(pile);
        Player.setTurn(true);
    }

    public void start() {
        input();
        play();
        loop();
    }

    BiConsumer p = (p,x)->{
            System.out.println("Enter name of Player1: ");
            player1 = new Player(at.next());
            System.out.println("Enter name of Player2: ");
            player2 = new Player(at.next());
    };


    @Override
    public void run() {
        while(play) {
            p.accept(player1, player2);
            start();
        }
    }
}




Player class.
package com.company;
import java.util.Scanner;

public class Player {

    private Scanner scan = new Scanner(System.in);
    private String name;
    private static boolean turn;
    private String in;
    private int s;

    public Player(){}
    public Player(String name){
        this.name = name;
        turn = true;
    }

    public static boolean isTurn() {
        return turn;
    }
    public static void setTurn(boolean turn) {
        Player.turn = turn;
    }


    public void pick(){
        System.out.println("Which pile do you want to pick from? " + name);
        in = scan.next();
        while(in.equals("A") && Game.getPile().get(0) == 0 ||in.equals("B")&& Game.getPile().get(1) == 0 ||in.equals("C")&& Game.getPile().get(2) == 0){
            System.out.println("Nice Try " + name + " that pile is empty try again." );
            in = scan.next();
        }
        choice();
    }

    @Override
    public String toString() {
        return "Pile A:" + Game.getPile().get(0) + "|Pile B:" + Game.getPile().get(1) + "|Pile C:" +Game.getPile().get(2);
    }

    public void choice(){
        switch(in){
            case "A":System.out.println("How many do you want to take out from pile A?");
                s = scan.nextInt();
                while(s > Game.getPile().get(0) || s == 0 || s < 0){
                    System.out.println("Pile A only has " + Game.getPile().get(0) + " try again.");
                    s = scan.nextInt();
                }
                Game.getPile().set(0,Game.getPile().get(0)-s);
                System.out.println(toString());
                break;
            case "B":System.out.println("How many do you want to take out from pile B?");
                s = scan.nextInt();
                while(s > Game.getPile().get(1) || s == 0 || s < 0){
                    System.out.println("Pile B only has " + Game.getPile().get(1) + " try again.");
                    s = scan.nextInt();
                }
                Game.getPile().set(1,Game.getPile().get(1)-s);
                System.out.println(toString());
                break;
            case "C":System.out.println("How many do you want to take out from pile C?");
                s = scan.nextInt();
                while(s > Game.getPile().get(2) || s == 0 || s < 0){
                    System.out.println("Pile C only has " + Game.getPile().get(2) + " try again.");
                    s = scan.nextInt();
                }
                Game.getPile().set(2,Game.getPile().get(2)-s);
                System.out.println(toString());
                break;
        }

    }

    public String getName() {
        return name;
    }

    public static boolean turn(){
        if(Game.getPile().get(0) == 0 && Game.getPile().get(1) == 0 && Game.getPile().get(2) == 0){
            setTurn(false);
        }else{
            setTurn(true);
        }
        return turn;
    }
    public static boolean mTurn(){
        if(Game.getPile().get(0) == 1 && Game.getPile().get(1) == 0 && Game.getPile().get(2) == 0){
            setTurn(false);
        }else if(Game.getPile().get(0) == 0 && Game.getPile().get(1) == 1 && Game.getPile().get(2) == 0){
            setTurn(false);
        }else if(Game.getPile().get(0) == 0 && Game.getPile().get(1) == 0 && Game.getPile().get(2) == 1){
            setTurn(false);
        }else{
            setTurn(true);
        }
        return turn;
    }
}



This post has been edited by g00se: 01 February 2017 - 07:29 AM
Reason for edit:: Fixed c tags


Is This A Good Question/Topic? 0
  • +

Replies To: I am new to programming and wanted to know how this code looked.

#2 NormR   User is offline

  • D.I.C Lover
  • member icon

Reputation: 765
  • View blog
  • Posts: 5,762
  • Joined: 25-December 13

Re: I am new to programming and wanted to know how this code looked.

Posted 01 February 2017 - 06:34 AM

The [CODE] tags have not been added correctly.
You need to use Preview before Submitting the post to verify that the code tags have been added correctly.

Posted Image

This post has been edited by NormR: 01 February 2017 - 06:36 AM

Was This Post Helpful? 1
  • +
  • -

#3 g00se   User is offline

  • D.I.C Lover
  • member icon

Reputation: 3622
  • View blog
  • Posts: 16,639
  • Joined: 20-September 08

Re: I am new to programming and wanted to know how this code looked.

Posted 01 February 2017 - 07:30 AM

Fixed code tags as decent attempt was made to use them
Was This Post Helpful? 1
  • +
  • -

#4 NormR   User is offline

  • D.I.C Lover
  • member icon

Reputation: 765
  • View blog
  • Posts: 5,762
  • Joined: 25-December 13

Re: I am new to programming and wanted to know how this code looked.

Posted 01 February 2017 - 08:54 AM

First thing to do is get rid of all static variables.
Was This Post Helpful? 0
  • +
  • -

#5 jon.kiparsky   User is offline

  • Beginner
  • member icon


Reputation: 11458
  • View blog
  • Posts: 19,524
  • Joined: 19-March 11

Re: I am new to programming and wanted to know how this code looked.

Posted 01 February 2017 - 09:33 AM

View PostNormR, on 01 February 2017 - 10:54 AM, said:

First thing to do is get rid of all static variables.


Always a good place to start, and a good learning exercise. Go through, pick each variable that's marked "static" and figure out how to make it non-static.

The while loop starting at line 44 of the Game class needs work. The guiding principle to start fiing it: Eliminate all tests of the loop variable within the loop.


Architecturally, Player should be an interface which you implement as a HumanPlayer and (if you want) as a MachinePlayer.
Was This Post Helpful? 0
  • +
  • -

#6 Rakin23   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 13
  • Joined: 01-February 17

Re: I am new to programming and wanted to know how this code looked.

Posted 01 February 2017 - 10:02 AM

I was thinking of that but was not sure how to do it.

This post has been edited by andrewsw: 03 February 2017 - 06:08 AM
Reason for edit:: removed quotes

Was This Post Helpful? 0
  • +
  • -

#7 Rakin23   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 13
  • Joined: 01-February 17

Re: I am new to programming and wanted to know how this code looked.

Posted 01 February 2017 - 10:50 AM

View PostNormR, on 01 February 2017 - 08:54 AM, said:

First thing to do is get rid of all static variables.


when i change Array list to non static i got a IndexOutOfBoundsException
Was This Post Helpful? 0
  • +
  • -

#8 NormR   User is offline

  • D.I.C Lover
  • member icon

Reputation: 765
  • View blog
  • Posts: 5,762
  • Joined: 25-December 13

Re: I am new to programming and wanted to know how this code looked.

Posted 01 February 2017 - 11:02 AM

Quote

when i change Array list to non static i got a IndexOutOfBoundsException

Changing to non-static should not cause that.

Please copy the text of the error message and paste it here.
Also post the new code that caused the error.
Was This Post Helpful? 0
  • +
  • -

#9 Rakin23   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 13
  • Joined: 01-February 17

Re: I am new to programming and wanted to know how this code looked.

Posted 03 February 2017 - 05:36 AM

View PostNormR, on 01 February 2017 - 11:02 AM, said:

Quote

when i change Array list to non static i got a IndexOutOfBoundsException

Changing to non-static should not cause that.

Please copy the text of the error message and paste it here.
Also post the new code that caused the error.


Quote

Sorry i couldn't reply sooner i had a lot of work i fixed the Exception and this is my new code


package com.company;

import java.util.ArrayList;
import java.util.Scanner;
import java.util.function.BiConsumer;

public class Game implements Runnable {
    private Scanner at = new Scanner(System.in);
    private boolean play;
    private ArrayList<Integer> pile;
    Player player1;
    Player player2;


    public Game(){
        pile = new ArrayList<>();
        play = true;
    }


    @Override
    public String toString() {
        return "Pile A:" + pile.get(0) + "|Pile B:" + pile.get(1) + "|Pile C:" + pile.get(2);
    }

    private void input(){
        System.out.println("How many sticks to put in pile A, B and C");
        System.out.println("Pile A");
        int in = at.nextInt();
        pile.add(in);
        System.out.println("Pile B");
        in = at.nextInt();
        pile.add(in);
        System.out.println("Pile C");
        in = at.nextInt();
        pile.add(in);
        System.out.println(toString());
    }
    public void play(){
        while (play) {
            player1.turn();
            if (!player1.isTurn()) {
                System.out.println(player2.getName() + " Has Won!!!");
                break;
            } else {
                player1.pick();
                player2.turn();
                if (!player2.isTurn()) {
                    System.out.println(player1.getName() + " Has Won!!!");
                    break;
                } else {
                    player2.pick();
                }
            }
        }
    }
    private void loop(){
        System.out.println("Do you want to play again?");
        if (at.next().equals("Yes")) {
            play = true;
        } else {
            play = false;
        }
    }

    public void start() {
        input();
        play();
        loop();
    }

    BiConsumer p = (p,x)->{
            System.out.println("Enter name of Player1: ");
            player1 = new Player(at.next(),pile);
            System.out.println("Enter name of Player2: ");
            player2 = new Player(at.next(),pile);
    };


    @Override
    public void run(){
        while(play) {
            p.accept(player1, player2);
            start();

        }
    }

}

package com.company;
import java.util.ArrayList;
import java.util.Scanner;

public class Player {
    private Scanner scan = new Scanner(System.in);
    private String name;
    private ArrayList<Integer> playPile;
    private boolean turn;
    private String in;
    private int s;

    public Player(){}
    public Player(String name, ArrayList<Integer> in){
        playPile = in;
        this.name = name;
        turn = true;
    }

    public boolean isTurn() {
        return turn;
    }
    public void setTurn(boolean turn) {
        this.turn = turn;
    }


    public void pick(){
        System.out.println("Which pile do you want to pick from? " + name);
        in = scan.next();
        while(in.equals("A") && playPile.get(0) == 0 ||in.equals("B")&& playPile.get(1) == 0 ||in.equals("C")&& playPile.get(2) == 0){
            System.out.println("Nice Try " + name + " that pile is empty try again." );
            in = scan.next();
        }
        choice();
    }

    @Override
    public String toString() {
        return "Pile A:" + playPile.get(0) + "|Pile B:" + playPile.get(1) + "|Pile C:" + playPile.get(2);
    }

    public void choice(){
        switch(in){
            case "A":System.out.println("How many do you want to take out from pile A?");
                s = scan.nextInt();
                while(s > playPile.get(0) || s == 0 || s < 0){
                    System.out.println("Pile A only has " + playPile.get(0) + " try again.");
                    s = scan.nextInt();
                }
                playPile.set(0,playPile.get(0)-s);
                System.out.println(toString());
                break;
            case "B":System.out.println("How many do you want to take out from pile B?");
                s = scan.nextInt();
                while(s > playPile.get(1) || s == 0 || s < 0){
                    System.out.println("Pile B only has " + playPile.get(1) + " try again.");
                    s = scan.nextInt();
                }
                playPile.set(1,playPile.get(1)-s);
                System.out.println(toString());
                break;
            case "C":System.out.println("How many do you want to take out from pile C?");
                s = scan.nextInt();
                while(s > playPile.get(2) || s == 0 || s < 0){
                    System.out.println("Pile C only has " + playPile.get(2) + " try again.");
                    s = scan.nextInt();
                }
                playPile.set(2,playPile.get(2)-s);
                System.out.println(toString());
                break;
        }

    }

    public String getName() {
        return name;
    }

    public boolean turn(){
        if(playPile.get(0) == 0 && playPile.get(1) == 0 && playPile.get(2) == 0){
            setTurn(false);
        }else{
            setTurn(true);
        }
        return turn;
    }
    public boolean mTurn(){
        if(playPile.get(0) == 1 && playPile.get(1) == 0 && playPile.get(2) == 0){
            setTurn(false);
        }else if(playPile.get(0) == 0 && playPile.get(1) == 1 && playPile.get(2) == 0){
            setTurn(false);
        }else if(playPile.get(0) == 0 && playPile.get(1) == 0 && playPile.get(2) == 1){
            setTurn(false);
        }else{
            setTurn(true);
        }
        return turn;
    }
}


Was This Post Helpful? 0
  • +
  • -

#10 NormR   User is offline

  • D.I.C Lover
  • member icon

Reputation: 765
  • View blog
  • Posts: 5,762
  • Joined: 25-December 13

Re: I am new to programming and wanted to know how this code looked.

Posted 03 February 2017 - 05:47 AM

Is the code working properly now?

This post has been edited by NormR: 03 February 2017 - 05:48 AM

Was This Post Helpful? 0
  • +
  • -

#11 Rakin23   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 13
  • Joined: 01-February 17

Re: I am new to programming and wanted to know how this code looked.

Posted 03 February 2017 - 06:00 AM

View PostNormR, on 03 February 2017 - 05:47 AM, said:

Is the code working properly now?

Quote

Yes it working.

Was This Post Helpful? 0
  • +
  • -

#12 andrewsw   User is offline

  • quantum multiprover
  • member icon

Reputation: 6776
  • View blog
  • Posts: 27,942
  • Joined: 12-December 12

Re: I am new to programming and wanted to know how this code looked.

Posted 03 February 2017 - 06:07 AM

You don't have to keep quoting (especially yourself). There is a large Reply button further down the page, or use the Fast Reply box.
Was This Post Helpful? 1
  • +
  • -

#13 CasiOo   User is offline

  • D.I.C Lover
  • member icon

Reputation: 1577
  • View blog
  • Posts: 3,551
  • Joined: 05-April 11

Re: I am new to programming and wanted to know how this code looked.

Posted 03 February 2017 - 01:15 PM

What I would like you to focus on is the Open-closed principle of SOLID. The principle says Objects or entities should be open for extension, but closed for modification.

How easy or difficult would it be to add a player 3 or a pile D to your game? Very difficult since you have to modify your existing Game and Player class.
For you to add a pile D, you would have to add another case to your switch. What if you had 200 pile choices, could you easily manage those, or would the code be difficult to maintain over time?
Any modification of existing code has a chance of introducing new bugs in what was before perfectly fine working code, so if we can get away with not touching the existing code, then that would be preferred.
I highly recommend you read about the SOLID principles.

I've tried looking at how pick and choice might be changed, trying to open them up for an unknown pile count. If there's then going to be a pile D, then you wouldn't have to change any of the code. You might also want to handle if the user tries to pick a non existing pile.
public Pile choosePile(PileCollection piles) {
    System.out.println("Which pile do you want to pick from? " + name);
    String userInput = scanner.next();
    Pile pile = piles.getPile(userInput);
    
    while (pile.isEmpty()) {
        System.out.println("Nice Try " + name + " that pile is empty try again.");
        userInput = scanner.next();
        pile = piles.getPile(userInput);
    }
    
    return pile;
}

public void takeSticksFromPile(Pile pile) {
    System.out.println("How many do you want to take out from pile " + pile.identifier + "?");
    int userInput = scanner.nextInt();
    
    while (pile.stickCount() < userInput) {
        System.out.println("Pile " + pile.identifier + " only has " + pile.stickCount() + " try again.");
    }
    
    pile.takeSticks(userInput);
}


Was This Post Helpful? 0
  • +
  • -

#14 Rakin23   User is offline

  • New D.I.C Head

Reputation: 0
  • View blog
  • Posts: 13
  • Joined: 01-February 17

Re: I am new to programming and wanted to know how this code looked.

Posted 03 February 2017 - 04:27 PM

Did you create a pile class casiOo
Was This Post Helpful? 0
  • +
  • -

#15 CasiOo   User is offline

  • D.I.C Lover
  • member icon

Reputation: 1577
  • View blog
  • Posts: 3,551
  • Joined: 05-April 11

Re: I am new to programming and wanted to know how this code looked.

Posted 03 February 2017 - 05:13 PM

If I were to code it, then I would have implemented two classes: PileCollection and Pile
Was This Post Helpful? 0
  • +
  • -

  • (2 Pages)
  • +
  • 1
  • 2