Code Review - Dice Game (No graphics)

This isn't an extensive list but I've tried to cover some of the major points.

Class length:
Break the code into classes; a single 600+ line class makes the code hard to follow. There's no hard rule on how long a class should be but each class should have one responsibility and should do it well. See this entry on the single responsibility principle for further information.

Method length:
While again there's no hard rule on how long a method should be, do try to refactor them by splitting them out into additional methods if they start to become too unwieldly. One clearly problematic method in your code is playGame().

Nesting depth:
You should do your best to avoid nesting code. Here's an example of what I mean by nesting:

for(int i = 0; i < 10; i++) {
    while(something) {
        if(something_else) {
            //do something
        } else {
            //do something else
        }
    }
}

This code is nested three levels deep and as soon as any real logic is added, not only will become difficult to understand but at some point in the future when you begin testing your code, you'll find it harder to test too.

At a glance, your playGame() is six levels deep at worst and has loops spanning dozens of lines. Try to refactor it so it's no deeper than one or two levels.

Simplify logic:
There are a few cases where your logic could be simplified. One instance is this:

        public static int setCurrentPlayer(int currentPlayer, int players)
        {
           if((currentPlayer+1)<=players)
            {
                currentPlayer+=1;
            }
            else if((currentPlayer+1)>players)
            {
                currentPlayer=1;
            }
            return currentPlayer;
       }

Replacing the else if with an else would achieve the same effect but would make the code easier to follow and hopefully save you from introducing buggy logic through adding conditions where they weren't needed. An if/else if doesn't inherently cover all possibilities but an if/else does.

Another example would be this piece of code in which the loop does nothing and could be safely removed.

for(int i=0;i<oldDice.length;i++)
{
    newDice=new int[oldDice.length];
}

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. ;)

Comments:
Good code should be self-documentating. While it's quite a subjective point, most of your comments provide no information that can't be gleaned by reading the method/argument names. If you feel a comment will be of value, use it to explain why you've chosen to implement functionality in a certain way, not how - that's what the code is there for.

Exceptions:
Handle the exceptions properly with try/catch at the point which they could be thrown, even if that means displaying an error and quitting. At the moment you're essentially just passing the buck up the call stack to methods that have nothing to do with the exceptions and shouldn't particularly need to concern themselves with them.

Formatting:
Your code formatting could be better. Think about the different sections of logic in your methods as paragraphs and add a blank line between each one.

Hope that's of some help to get you started.

Edit: Lost connection during posting, came out slightly broken.

Indifferent,

Thank you so much for your review. Just by looking at my code, I knew that there were a ton of things that could have been done better. Most of the things you pointed out were the issues that I knew needed to be fixed, but wasn't sure how. I highly appreciate you taking the time to review it, and more importantly, that you were nice about it and offered me some solutions. The biggest thing that I would like to learn is coding etiquette and formatting. Do you happen to know of any websites or articles that offer any insight into this?

Perfection is achieved, not when there is nothing more to add, but when there is nothing left to take away. ;)

Thank you for this

Indifferent,

Thank you so much for your review. Just by looking at my code, I knew that there were a ton of things that could have been done better. Most of the things you pointed out were the issues that I knew needed to be fixed, but wasn't sure how. I highly appreciate you taking the time to review it, and more importantly, that you were nice about it and offered me some solutions. The biggest thing that I would like to learn is coding etiquette and formatting. Do you happen to know of any websites or articles that offer any insight into this?

There are quite a few books that you'll see recommended time and time again. Code Complete seems to be a seminal book that might interest you, although I haven't read it myself so I can't vouch for it.

I personally just tend to read articles that I stumble across, search for recommendations when I'm unsure whether I'm going about something in the right way (almost every question has been asked and answered a dozen times already) or browse forums like this one.

I agree with most of the things Indifferent says, but I want to add a few small things.

All methods are static. Basically this means there is no Object Oriented programming, which makes it harder to break things apart. I think it would be a good idea to try and find a book that discusses Java and OO.

Logic and UI are not separated. This is along the lines with Indifferents "Class length", but with a different angle.. When you separate the logic from the actual UI output it will be easier later on to try and add a Graphical layer, besides that it will be easier to test your logic using unit tests.

In the rules() method you have multiple calls to System.out.println(), and although it is probably a matter of taste, I would combine the complete string and print it all in one println().

Btw. for method lengths a good rule of thumb is methods between 3 and 7 lines, especially when the logic is complex, you want less lines.

Thanks, both of you, for the advice on using classes. I'm currently reading through a book titled "Java Programming: From the Ground Up", and have just gotten to the basics of OOP. In fact, I think the chapter I'm about to start is about making classes and implementing them correctly.

Btw. for method lengths a good rule of thumb is methods between 3 and 7 lines, especially when the logic is complex, you want less lines.

I'm always afraid that I'm going to have too many methods in my program, so would I clear this up by making custom classes, and then just creating an object that can reference them?

Code Complete seems to be a seminal book that might interest you, although I haven't read it myself so I can't vouch for it.

Thanks for this! I'll look it up on Amazon and give it a whirl.

Btw. for method lengths a good rule of thumb is methods between 3 and 7 lines, especially when the logic is complex, you want less lines.

I'm always afraid that I'm going to have too many methods in my program, so would I clear this up by making custom classes, and then just creating an object that can reference them?

I don't think you can have too many methods in a program. As is said before, your class should have one responsibility, but your methods shoud also have just one responsibility. Besides that, not all methods have to be public.

That being said, sometimes there are reasons to make a method larger than usually. This could be for readability or performance.

I can recommend the articles, videos and book of Robert C. Martin about "Clean Code". It is a relative old book, but still very usefull to read.

I can recommend the articles, videos and book of Robert C. Martin about "Clean Code". It is a relative old book, but still very usefull to read.

Thank you so much for this. I know that this code isn't very elaborate, nor is the game. But, I really appreciate the objectiveness you have shown me. I know that sometimes, newbies (like me) are easy to tear apart, since their code is messy and wrong. You guys have really helped me to feel welcome to GameDevs. Thank you for all of your advice.

Btw. for method lengths a good rule of thumb is methods between 3 and 7 lines, especially when the logic is complex, you want less lines.

I'm always afraid that I'm going to have too many methods in my program, so would I clear this up by making custom classes, and then just creating an object that can reference them?

I don't think you can have too many methods in a program. As is said before, your class should have one responsibility, but your methods shoud also have just one responsibility. Besides that, not all methods have to be public.

That being said, sometimes there are reasons to make a method larger than usually. This could be for readability or performance.

I can recommend the articles, videos and book of Robert C. Martin about "Clean Code". It is a relative old book, but still very usefull to read.

This might help... you don't always have to think of methods as performing some vital function... sometimes you can have a private method whose sole purpose is to simplify the code in a more complicated public method. I often write little private methods that do various bits of non-trivial math operations, just to simplify the code.

This might help... you don't always have to think of methods as performing some vital function... sometimes you can have a private method whose sole purpose is to simplify the code in a more complicated public method. I often write little private methods that do various bits of non-trivial math operations, just to simplify the code.

Agreed, sometimes I make a method just for the condition of an if statement, especially when it is more complex or used multiple times.

Agreed, sometimes I make a method just for the condition of an if statement, especially when it is more complex or used multiple times.

I'm starting to realize this. I'm big about "clearing" the screen whenever changing player turns. I realized that it was a lot simpler to write a small method named clearScreen(), rather than type out:

for(int i=0;i<10;i++)
{
  System.out.println();
}

every single time that I wanted to clear the screen.

I know that using global variables is usually frowned upon, but is it okay to use them if I believe that I can do so efficiently?

I know that using global variables is usually frowned upon, but is it okay to use them if I believe that I can do so efficiently?

There are no hard and fast rules in programming, even when it comes to some of the more controversial constructs. Although a global or two likely won't hurt, you should get into the habit of doing what you can to avoid needing them.