6
\$\begingroup\$

I am working on writing a Code Golf King of the Hill challenge, the details of which can be read here. You can read the full code here.

Briefly, the game proceeds in cycles of night then day. There are two mafia, who collectively kill one person at night, and five town sided players (some of whom have roles during the night, not important here.) during the day, in human version all people living discuss to try to figure out who is who. The mafia want to kill all the towns people but the townspeople want to eliminate the mafia. Therefore, after discussing the occurrences of the night, they vote to lynch a person (or not to), the objective being to eliminate a mafia. By making false claims, the mafia can pretend to be town, but if they mess up the town should notice. The fun of the game is that not everyone is telling the truth. The link above has a more in depth explanation.

Notice that each person has to claim a role publicly, and they can claim to be one role while actually bring a different one. Therefore I have two identities, one actual and one public. They can be contradictory or in agreement, depending on the players strategy.

Basically, the users will program Java bots that will play the game, given only the information provided by other players and sometimes from the controller program. This information is provided by a static ArrayList of the Identity objects of all players, specifically the public Identity object, since each player has two Identities.

I specifically want to ask about how my class structure and object inheritance structure is set up, whether it should be changed, whether users can easily use it and understand what is going on, possible information leaks, etc. I also want some advice about initializing the game in my controller class, and especially how I am going to load all the users responses into the program and randomly select some of their strategies to be put to the test.

I have put ... in place of some methods which aren't terribly important for this question.

Here is the Player class:

public abstract class Player {
    ...
    static ArrayList<Identity> Identities = new ArrayList<Identity>();
    public static int numTown, numMafia;
    public int hat, role; //hat is basically the players ID number, unrelated to the players actual role in the game. 
    Player otherMafia;
    Identity pub, priv;
    String Role;

    public Player() {...
    }

    public final int day(int i) {
        if (pub.life) {
            if ((role == 1) || (role == 2))
                return dayMafia();
            if (role == 3)
                return dayCop();
            if (role == 4)
                return dayDoctor();
            else
                return dayTown();
        } else
            return 0;
    }

    public abstract int dayCop();

    public abstract int dayDoctor();

    public abstract int dayMafia();

    public abstract int dayTown();

    public final int night(int i, int mafiaChoice) {
        if (pub.life) {
            if ((role == 1) || (role == 2))
                    return nightMafia(i, mafiaChoice);
            if (role == 3)
                return nightCop();
            if (role == 4)
                return nightDoctor();
            else
                return 0;
        } else if ((role == 1) || (role == 2))
            return mafiaChoice;
        else return 0;
    }

    public abstract int nightCop();

    public abstract int nightDoctor();

    public abstract int nightMafia(int i, int mafiaChoice);

    ... //Various methods such as isPlayerDead, 
    ... //etc. which will give players information
    ... //about other players statuses easily. 
}

And here is the Identity class:

import java.util.ArrayList;

public class Identity {
    int hat;
    boolean life;
    int lynch;
    int role;
    String Role;
    ArrayList<String> suspicions, visits;

    // life = 0 means dead; life = 1 means alive

    public Identity() {
        suspicions = new ArrayList<String>();
        visits = new ArrayList<String>();
        role = 0;
        life = true;
    }

    @Override
    public String toString() {...}
}

And then here's part of the controller program which initializes the whole game by assigning every player their roles, adding their public Identity objects to the Arraylist, and gives them their hats. Here is the relevant excerpt that I would like some advice on. (TestPlayer is another test player class, and for testing purposes all of them share the same bad strategy. No problems there!)

Player p1 = new TestPlayer(), p2 = new TestPlayer(), p3 = new TestPlayer();
    Player p4 = new TestPlayer(), p5 = new TestPlayer(), p6 = new TestPlayer(), p7 = new TestPlayer();
    ArrayList<Player> players = new ArrayList<Player>();
    players.add(p1);
    players.add(p2);
    players.add(p3);
    players.add(p4);
    players.add(p5);
    players.add(p6);
    players.add(p7);
    Collections.shuffle(players, new Random(System.nanoTime())); // shuffle  around players

    ArrayList<Integer> roles = new ArrayList<Integer>();
    roles.add(1);
    roles.add(2);
    roles.add(3);
    roles.add(4);
    roles.add(5);
    roles.add(6);
    roles.add(7);
    Collections.shuffle(roles, new Random(System.nanoTime())); // shuffle around roles

    Player M1 = null, M2 = null, C = null, D = null;
    ArrayList<Identity> Ids = new ArrayList<Identity>();

    for (int i = 0; i < 7; i++) { //assign roles to all players
        players.get(i).setHat(i + 1);
        players.get(i).setRole(roles.get(i));
        if (roles.get(i) == 1)
            M1 = players.get(i);
        if (roles.get(i) == 2)
            M2 = players.get(i);
        if (roles.get(i) == 3)
            C = players.get(i);
        if (roles.get(i) == 4)
            D = players.get(i);
        Ids.add(players.get(i).pub);
        System.out.println("hat #" + (i + 1) + " got role "
                + players.get(i).role + " " + players.get(i).Role);
    }
    Player.Identities = Ids;

    M1.otherMafia = M2;
    M2.otherMafia = M1;
\$\endgroup\$
  • 1
    \$\begingroup\$ What is the goal and the winning criterion? \$\endgroup\$ – 200_success Jun 24 '15 at 21:18
  • \$\begingroup\$ Is each challenge participant to implement the whole town, or will each bot be grouped with six other submissions? \$\endgroup\$ – 200_success Jun 24 '15 at 21:23
  • \$\begingroup\$ @200_success the bot will be paired with other bots, so 7 different bots compete in one game, either 5 win or 5 lose. I will add some explanation of game mechanics in the question \$\endgroup\$ – Faraz Masroor Jun 24 '15 at 21:57
  • \$\begingroup\$ How do you propose to load 7 different Player implementations in the same JVM process? \$\endgroup\$ – 200_success Jun 25 '15 at 0:23
  • \$\begingroup\$ @200_success That's one of my questions, i mean if we have Player1.class etc. then I can just do new Player1(), new Player2(), etc. but I'll need a better way to do this for more responses. \$\endgroup\$ – Faraz Masroor Jun 25 '15 at 0:25
8
\$\begingroup\$

I specifically want to ask about how my class structure and object inheritance structure is set up, whether it should be changed

A generic Player class with methods like dayCop, dayDoctor, dayMafia sounds like it would be better to have Cop, Doctor, Mafia sub-classes and a day method defined in the parent Player and overridden in the sub-classes.

Except that "day" is not a good name, as it doesn't suggest the purpose (just like "dayCop", "dayDoctor", etc, don't either).

[...] whether users can easily use it and understand what is going on

I cannot. The class names and method names don't really mean anything, it all seems very abstract.

[...] and especially how I am going to load all the users responses into the program and randomly select some of their strategies to be put to the test.

It's interesting that you mention the term "strategies". The Strategy design pattern looks like a very good fit for your situation. There can be a general behavior in your program that works with Player instances, and specialized behavior that is implemented in the sub-classes. For example it seems you can have a DayStrategy and a NightStrategy.

Bad practices

Several bad practices jump in the eyes:

  • Refer to types by interfaces instead of implementation. For example, declare variables as List instead of ArrayList
  • Make member variables private final or private if possible
  • Avoid repeated hash table lookups like roles.get(i). Cache in a local variable and reuse
  • Use else if between if statements that cannot be true at the same time
  • Variables should be named camelCase (violated by "Identities" and "Ids")
\$\endgroup\$
  • \$\begingroup\$ Perhaps I should have added a bit more explanation on the game mechanics and what the methods mean. Please read again in a few minutes. \$\endgroup\$ – Faraz Masroor Jun 24 '15 at 20:24
  • 2
    \$\begingroup\$ Ideally the code should speak for itself. \$\endgroup\$ – janos Jun 24 '15 at 20:26
  • \$\begingroup\$ I added an explanation of game mechanics. In code golf everyone is supposed to read the explanation first before reading the challenge or code. I wanted to make one class because in real life a person playing the game will not know what role he will get, so users answering my challenge will have to define all strategies beforehand. Also reduces never if files. \$\endgroup\$ – Faraz Masroor Jun 24 '15 at 22:06
  • \$\begingroup\$ I don't really understand how I should use this Strategy thing you brought up, I meant that each user defined the methods his own way and I just call on them. I was asking how, if I have 20 responses, how my source code will pick seven of them to have a game of Mafia. \$\endgroup\$ – Faraz Masroor Jun 25 '15 at 0:26
  • 1
    \$\begingroup\$ I want each user to only have to write one class. Are you suggesting making Player an interface to be implemented? \$\endgroup\$ – Faraz Masroor Jun 25 '15 at 17:14

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.