4
\$\begingroup\$

The game will be combination of text adventure (navigate through buttons), RPG, sandbox, strategy and turn-based game where the main task of player is to explore the game's world and survive (since I cannot classify my game with one genre just yet, I'm still experimenting).

The interaction between player and game, as I said, will be based on button clicks. The player will be able to go to different locations using tabs, check some statistics ("Attributes") and perform actions in each location to advance in the game.

I've got a code that compiles, and it works the way I want. However, I wondered if I can do anything to make process of programming-in more Actions easier and 'cheaper' in terms of time&effort I have to put into each button, since I want to have a LOT of actions available for player.

Currently I am using this method below to display Actions to player and I'm calling it on the start of each turn to refresh the whole layout and display Actions that are available to player (based on his Resources and his Achievements so far).

The code is much longer but it is pretty similar with each next action in the 'database' of sorts.

Do I have any options to reduce the size of the code needed to make another button (considering I have a few if statements here and there), and/or reduce time I have to put into adding more actions into the game?

Because as I add more actions (and I plan to have way over 100 actions at some point, of course not all will be displayed to player) the code is getting really long and really littered due to the space being taken JUST by this single method (displayactivity()) in my MainActivity.java.

public void displayactions(){
    setContentView(R.layout.main_game);
    displaytabs();
    displayturns();
    textView = (TextView) findViewById(R.id.workers);
    switch(people){
    case 1:
        textView.setText(workers+"/"+people+" worker available.");
    break;
    default:
        textView.setText(workers+"/"+people+" workers available.");
    }
    textView = (TextView) findViewById(R.id.gamelogs);
    textView.setText(sb);
    LinearLayout layoutmain1 = (LinearLayout) findViewById(R.id.actions);

    switch (tabs){

    case 2:

    if(true){
    Button button = new Button(this);
    button.setLayoutParams(new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT));
    button.setText("Collect wood | Cost: 1 ["+workers+"] Worker & 1 ["+food+"] Food | Effect: +2-5 Materials");
    button.setId(201);
    button.setTextSize(15);

    if(food >= 1 && workers >= 1){
    button.setTextColor(getResources().getColor(R.color.black));
    } else {
    button.setEnabled(false);
    button.setTextColor(getResources().getColor(R.color.grey));
    }

    button.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View v) {
            food -= 1;
            workers -= 1;
            int found = getrandom(2, 5);
            materials += found;
            displayactions();
            sb.insert(0,"There are lots of branches to collect in the forest. You find "+found+" materials.\n");
            textView = (TextView) findViewById(R.id.gamelogs);
            textView.setText(sb);
        }
    });
    layoutmain1.addView(button);
    }

    if(true){
    Button button = new Button(this);
    button.setLayoutParams(new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT));
    button.setText("Go hunting | Cost: 1 ["+workers+"] Worker | Effect: +1-2 Food");
    button.setId(202);
    button.setTextSize(15);

    if(workers >= 1){
        button.setTextColor(getResources().getColor(R.color.black));
    }else{
        button.setEnabled(false);
        button.setTextColor(getResources().getColor(R.color.grey));
    }

    button.setOnClickListener(new View.OnClickListener() {
        @Override
        public void onClick(View v) {
            workers -= 1;
            int found = getrandom(1, 2);
            food += found;
            displayactions();
            //Button button = (Button) findViewById(202);
            //button.setEnabled(false);
            //button.setTextColor(getResources().getColor(R.color.grey));
            switch(found){
            case 1:
                sb.insert(0,"Wilderness feeds you. Your hunting efforts bring "+found+" portion of food.\n");
            break;
            default:
                sb.insert(0,"Wilderness feeds you. Your hunting efforts bring "+found+" portions of food.\n");
            }
            textView = (TextView) findViewById(R.id.gamelogs);
            textView.setText(sb);
        }
    });
    layoutmain1.addView(button);
    }

    if(constructor == 0 && campfire == 1 && tent == 1){
        Button button = new Button(this);
        button.setLayoutParams(new LayoutParams(LayoutParams.MATCH_PARENT, LayoutParams.WRAP_CONTENT));
        button.setText("Hire a Constructor | Cost: 1 ["+workers+"] Worker & 10 ["+food+"] Food | Effect: A good man, who only desires food and a shelter.");
        button.setId(203);
        button.setTextSize(15);

    if(workers >= 1 && food >= 10){
        button.setTextColor(getResources().getColor(R.color.black));
    }else{
        button.setEnabled(false);
        button.setTextColor(getResources().getColor(R.color.grey));
    }

        button.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                workers -= 1;
                food -= 10;
                constructor += 1;
                people += 1;
                displayactions();
                sb.insert(0,"You welcome the Constructor.\n");
                textView = (TextView) findViewById(R.id.gamelogs);
                textView.setText(sb);
            }
        });
        layoutmain1.addView(button);
        }
    }
    }
\$\endgroup\$
  • \$\begingroup\$ One problem jumps out if(true) is a useless if statement that causes clutter. Remove it. \$\endgroup\$ – Jeff Vanzella Dec 5 '13 at 22:11
  • \$\begingroup\$ Yea i know that if(true) is unnecessary, I simply put it there so I don't forget to revise the requirements of these 'Actions'. I could use brackets {} for now to separate the code. Thanks for your suggestion, I'll try to keep it in mind. \$\endgroup\$ – Dravic Dec 6 '13 at 15:33
4
\$\begingroup\$

When reading your code, the Strategy Pattern alarm goes off in my head. Before that however, I have some other remarks.

  • Indentation. If you are using Eclipse, please select your code and press Ctrl + I to indent your code properly. Please read up on indentation in the Java Coding Conventions

  • Spacing. This can be seen as a very minor thing, but when you think about it it makes sense that you should be consistent in the usage of spacing around braces; in Java, it is convention to always use a space before a {.

  • As stated by Jeff Vanzella in a comment to your question, if(true) { is an unnecessary condition. If you use a different block to separate variable scoping to avoid variable naming collisions, you can simply use { }-bracers without an if-condition. Or you might want to extract a method that is doing that part. Or, you might want to remove the extra block entirely. (without removing the code, of course)

  • Duplicate as small strings as possible. Instead of using two separate long strings, use one string for portion and one for portions. One string for worker and one for workers. Use Android String Resources, which even supports plurals, instead of hard-coding strings. It is quite neat to gather all strings in one place, and it makes internationalization so much easier.

Now, about that strategy pattern...

(This is only an example on how to do it, there are many other ways. Use the things that fits you. The important thing is that you understand the overall structure of what I am doing below)

Something that can improve the cleanliness of your code is to put the game variables in one object of a Game class; this could contain variables such as food, workers, materials and so on.

Then create a GameAction interface.

public interface GameAction {
    // You might not want all these methods, perhaps you only want one method for returning a Button
    // and one for updating the Button status or something similar. Use what fits you.
    boolean isAllowed(Game game);
    String getText(Game game);
    void execute(Game game);
}

Then create a List<GameAction> actionList;

Create some implementations of GameAction (possibly one for each action, or some actions might be possible to group together).

public class CollectBranchesAction implements GameAction {
     @Override
     public boolean isAllowed(Game game) {
         // This action is allowed if we have at least 1 food and at least 1 worker.
         return game.getFood() > 0 && game.getWorkers() > 0;
     }

     @Override
     public String getText(Game game) {
         return "Collect wood | Cost: 1 [" + game.getWorkers() + "] Worker & 1 [" + game.getFood() + "] Food | Effect: +2-5 Materials";
     }

     @Override
     public void execute(Game game) {
         game.changeFood(-1);
         game.changeWorkers(-1);
         int found = getrandom(2, 5);
         game.changeMaterials(found);
         game.addText("There are lots of branches to collect in the forest. You find "+found+" materials.\n");
     }
}

Initialize the list, and add an instance of CollectBranchesAction to the list:

List<GameAction> actionList = new ArrayList<GameAction>();
actionList.add(new CollectBranchesAction());

Loop through the list, and create buttons for the list - and call the GameAction methods to determine the state of the button.

for (final GameAction action : actionList) {
     Button button = new Button(this);
     button.setText(action.getText(game));
     button.setEnabled(action.isAllowed(game));
     button.setOnClickListener(new View.OnClickListener() {
         @Override
         public void onClick(View v) {
              action.execute(game);
         }
     });
     layoutmain1.addView(button);
}

I hope this will get you started in the right direction!

\$\endgroup\$
  • \$\begingroup\$ Thank you a lot for help! =) I appreciate the time you've spent making this post for me. I'll now re-read it few times and try to make sense of it. Then I will try to implement it in my code and i'll get back here as fast as possible with any further questions. \$\endgroup\$ – Dravic Dec 6 '13 at 15:31
  • \$\begingroup\$ I'm fairly new to programming so I still make such silly mistakes. I do not have the knowledge you guys have, and am still learning. I really appreciate all the suggestions you've put into your post and I will attempt to stick to them. I seriously didn't even think of making separate class for each Action even though now that you've said it, it makes perfect sense. Simon, would I be pushing it if I made sense of your post, then applied it to my code and then reposted ALL of my code (likely 500 lines or so) somewhere so you can revise all of it and perhaps suggest anything else? \$\endgroup\$ – Dravic Dec 6 '13 at 15:36
  • \$\begingroup\$ @Dravic Sure, I think I can be able to go through 500 lines. I would like that when you post it, you can explain your code a bit (similar to what you have been doing now). That makes it easier for us reviewers :) \$\endgroup\$ – Simon Forsberg Dec 6 '13 at 16:18
  • \$\begingroup\$ @Dravic By the way, the reason for why I am suggesting this approach is that I have done the exact same mistakes that you have :) \$\endgroup\$ – Simon Forsberg Dec 6 '13 at 16:18
  • \$\begingroup\$ :) Please check out @OP, i edited it and added another block of code, this time with my save and load methods. In answer to this: "Something that can improve the cleanliness of your code is to put the game variables in one object of a Game class; this could contain variables such as food, workers, materials and so on." - see, I have a problem because i need a new way to save my stuff. currently i used SharedPreferences for saving primitive fields but now, with objects, i am not sure how do i save this 'object with variables inside'. :/ \$\endgroup\$ – Dravic Dec 6 '13 at 16:48

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.