Skip to main content

OOP

This task really begs for object oriented programming.

As a first step, I

I suggest to movemoving the maxhp, playerhp, playermeleedmg, xp, level fields to a dedicated class for players, and and rewrite the build* methods. You could create a PlayerFactory with the build* methods that create and return warriors, archers, mages, enemies. All these different kind of characters could inherit from a common base Player class.

Rolling the dice

To get a random number x in the inclusive range of [1:6], instead of rand.nextInt(7) and skipping zeros in a while loop, you should use 1 + rand.nextInt(6) to get the same effect.

The roll6, roll10, roll20 methods all use the same logic. Just one roll method with a parameter could do the job:

public class Dicedice {
    private final Random random = new Random();

    public int roll(int max) {
        return 1 + random.nextInt(max);
    }
    
    public int roll6() {
        return roll(6);
    }
    
    // and so on ...
}

As demonstrated in this example, you probably don't need a new Random instance before each roll.

Chaining conditions

These conditions are mutually exclusive, therefore they should be chained with else if rather than using multiple independent if statements:

if(charclass.charAt(0) == 'w'){
    buildWarrior();
}
if(charclass.charAt(0) == 'a'){
    buildArcher();
}
if(charclass.charAt(0) == 'm'){
    buildMage();
}

In this example a switch would be even better, so that charclass.charAt(0) will only be evaluated once.

Don't repeat yourself

There are near-duplicate lines at multiple places. For example the enemyattack and attack methods implement the same logic, with only minor differences in the details. Those details can be parameters, so that you can avoid copy-pasting code. Later when you need to update copy-pasted sections, it will be extremely annoying to make parallel changes at multiple locations, and it will be very error-prone too. Probably 99% of the time it's better to extract common logic and generalize than to copy-paste.

Formatting

You're using Eclipse Luna, it has a feature to reformat nicely the entire code. It's good to use that until it becomes a natural habit to type nicely.

OOP

This task really begs for object oriented programming.

As a first step, I suggest to move the maxhp, playerhp, playermeleedmg, xp, level fields to a dedicated class for players, and rewrite the build* methods. You could create a PlayerFactory with the build* methods that create and return warriors, archers, mages, enemies. All these different kind of characters could inherit from a common base Player class.

Rolling the dice

To get a random number x in the inclusive range of [1:6], instead of rand.nextInt(7) and skipping zeros in a while loop, you should use 1 + rand.nextInt(6) to get the same effect.

The roll6, roll10, roll20 methods all use the same logic. Just one roll method with a parameter could do the job:

public class Dice {
    private final Random random = new Random();

    public int roll(int max) {
        return 1 + random.nextInt(max);
    }
    
    public int roll6() {
        return roll(6);
    }
    
    // and so on ...
}

As demonstrated in this example, you probably don't need a new Random instance before each roll.

Chaining conditions

These conditions are mutually exclusive, therefore they should be chained with else if rather than using multiple independent if statements:

if(charclass.charAt(0) == 'w'){
    buildWarrior();
}
if(charclass.charAt(0) == 'a'){
    buildArcher();
}
if(charclass.charAt(0) == 'm'){
    buildMage();
}

In this example a switch would be even better, so that charclass.charAt(0) will only be evaluated once.

Don't repeat yourself

There are near-duplicate lines at multiple places. For example the enemyattack and attack methods implement the same logic, with only minor differences in the details. Those details can be parameters, so that you can avoid copy-pasting code. Later when you need to update copy-pasted sections, it will be extremely annoying to make parallel changes at multiple locations, and it will be very error-prone too. Probably 99% of the time it's better to extract common logic and generalize than to copy-paste.

Formatting

You're using Eclipse Luna, it has a feature to reformat nicely the entire code. It's good to use that until it becomes a natural habit to type nicely.

OOP

This task really begs for object oriented programming.

As a first step,

I suggest moving the maxhp, playerhp, playermeleedmg, xp, level fields to a dedicated class for players, and rewrite the build* methods. You could create a PlayerFactory with the build* methods that create and return warriors, archers, mages, enemies. All these different kind of characters could inherit from a common base Player class.

Rolling the dice

To get a random number x in the inclusive range of [1:6], instead of rand.nextInt(7) and skipping zeros in a while loop, you should use 1 + rand.nextInt(6) to get the same effect.

The roll6, roll10, roll20 methods all use the same logic. Just one roll method with a parameter could do the job:

public class dice {
    private final Random random = new Random();

    public int roll(int max) {
        return 1 + random.nextInt(max);
    }
    
    public int roll6() {
        return roll(6);
    }
    
    // and so on ...
}

As demonstrated in this example, you probably don't need a new Random instance before each roll.

Chaining conditions

These conditions are mutually exclusive, therefore they should be chained with else if rather than using multiple independent if statements:

if(charclass.charAt(0) == 'w'){
    buildWarrior();
}
if(charclass.charAt(0) == 'a'){
    buildArcher();
}
if(charclass.charAt(0) == 'm'){
    buildMage();
}

In this example a switch would be even better, so that charclass.charAt(0) will only be evaluated once.

Don't repeat yourself

There are near-duplicate lines at multiple places. For example the enemyattack and attack methods implement the same logic, with only minor differences in the details. Those details can be parameters, so that you can avoid copy-pasting code. Later when you need to update copy-pasted sections, it will be extremely annoying to make parallel changes at multiple locations, and it will be very error-prone too. Probably 99% of the time it's better to extract common logic and generalize than to copy-paste.

Formatting

You're using Eclipse Luna, it has a feature to reformat nicely the entire code. It's good to use that until it becomes a natural habit to type nicely.

added 112 characters in body
Source Link
janos
  • 113.1k
  • 15
  • 154
  • 396

OOP

This task really begs for object oriented programming.

As a first step, I suggest to move the maxhp, playerhp, playermeleedmg, xp, level fields to a dedicated class for players, and rewrite the build* methods. You could create a PlayerFactory with the build* methods that create and return warriors, archers, mages, enemies. All these different kind of characters could inherit from a common base Player class.

Rolling the dice

To get a random number x in the inclusive range of [1:6], instead of rand.nextInt(7) and skipping zeros in a while loop, you should use 1 + rand.nextInt(6) to get the same effect.

The roll6, roll10, roll20 methods all use the same logic. Just one roll method with a parameter could do the job:

public class Dice {
    private final Random random = new Random();

    public int roll(int max) {
        return 1 + random.nextInt(max);
    }
    
    public int roll6() {
        return roll(6);
    }
    
    // and so on ...
}

As demonstrated in this example, you probably don't need a new Random instance before each roll.

Chaining conditions

These conditions are mutually exclusive, therefore they should be chained with else if rather than using multiple independent if statements:

if(charclass.charAt(0) == 'w'){
    buildWarrior();
}
if(charclass.charAt(0) == 'a'){
    buildArcher();
}
if(charclass.charAt(0) == 'm'){
    buildMage();
}

In this example a switch would be even better, so that charclass.charAt(0) will only be evaluated once.

Don't repeat yourself

There are near-duplicate lines at multiple places. For example the enemyattack and attack methods implement the same logic, with only minor differences in the details. Those details can be parameters, so that you can avoid copy-pasting code. Later when you need to update copy-pasted sections, it will be extremely annoying to make parallel changes at multiple locations, and it will be very error-prone too. Probably 99% of the time it's better to extract common logic and generalize than to copy-paste.

Formatting

You're using Eclipse Luna, it has a feature to reformat nicely the entire code. It's good to use that until it becomes a natural habit to type nicely.

OOP

This task really begs for object oriented programming.

As a first step, I suggest to move the maxhp, playerhp, playermeleedmg, xp, level fields to a dedicated class for players, and rewrite the build* methods. You could create a PlayerFactory with the build* methods that create and return warriors, archers, mages, enemies. All these different kind of characters could inherit from a common base Player class.

Rolling the dice

To get a random number x in the inclusive range of [1:6], instead of rand.nextInt(7) and skipping zeros in a while loop, you should use 1 + rand.nextInt(6) to get the same effect.

The roll6, roll10, roll20 methods all use the same logic. Just one roll method with a parameter could do the job:

public class Dice {
    private final Random random = new Random();

    public int roll(int max) {
        return 1 + random.nextInt(max);
    }
    
    public int roll6() {
        return roll(6);
    }
    
    // and so on ...
}

As demonstrated in this example, you probably don't need a new Random instance before each roll.

Chaining conditions

These conditions are mutually exclusive, therefore they should be chained with else if rather than using multiple independent if statements:

if(charclass.charAt(0) == 'w'){
    buildWarrior();
}
if(charclass.charAt(0) == 'a'){
    buildArcher();
}
if(charclass.charAt(0) == 'm'){
    buildMage();
}

Don't repeat yourself

There are near-duplicate lines at multiple places. For example the enemyattack and attack methods implement the same logic, with only minor differences in the details. Those details can be parameters, so that you can avoid copy-pasting code. Later when you need to update copy-pasted sections, it will be extremely annoying to make parallel changes at multiple locations, and it will be very error-prone too. Probably 99% of the time it's better to extract common logic and generalize than to copy-paste.

Formatting

You're using Eclipse Luna, it has a feature to reformat nicely the entire code. It's good to use that until it becomes a natural habit to type nicely.

OOP

This task really begs for object oriented programming.

As a first step, I suggest to move the maxhp, playerhp, playermeleedmg, xp, level fields to a dedicated class for players, and rewrite the build* methods. You could create a PlayerFactory with the build* methods that create and return warriors, archers, mages, enemies. All these different kind of characters could inherit from a common base Player class.

Rolling the dice

To get a random number x in the inclusive range of [1:6], instead of rand.nextInt(7) and skipping zeros in a while loop, you should use 1 + rand.nextInt(6) to get the same effect.

The roll6, roll10, roll20 methods all use the same logic. Just one roll method with a parameter could do the job:

public class Dice {
    private final Random random = new Random();

    public int roll(int max) {
        return 1 + random.nextInt(max);
    }
    
    public int roll6() {
        return roll(6);
    }
    
    // and so on ...
}

As demonstrated in this example, you probably don't need a new Random instance before each roll.

Chaining conditions

These conditions are mutually exclusive, therefore they should be chained with else if rather than using multiple independent if statements:

if(charclass.charAt(0) == 'w'){
    buildWarrior();
}
if(charclass.charAt(0) == 'a'){
    buildArcher();
}
if(charclass.charAt(0) == 'm'){
    buildMage();
}

In this example a switch would be even better, so that charclass.charAt(0) will only be evaluated once.

Don't repeat yourself

There are near-duplicate lines at multiple places. For example the enemyattack and attack methods implement the same logic, with only minor differences in the details. Those details can be parameters, so that you can avoid copy-pasting code. Later when you need to update copy-pasted sections, it will be extremely annoying to make parallel changes at multiple locations, and it will be very error-prone too. Probably 99% of the time it's better to extract common logic and generalize than to copy-paste.

Formatting

You're using Eclipse Luna, it has a feature to reformat nicely the entire code. It's good to use that until it becomes a natural habit to type nicely.

hid internal state of Dice class
Source Link

OOP

This task really begs for object oriented programming.

As a first step, I suggest to move the maxhp, playerhp, playermeleedmg, xp, level fields to a dedicated class for players, and rewrite the build* methods. You could create a PlayerFactory with the build* methods that create and return warriors, archers, mages, enemies. All these different kind of characters could inherit from a common base Player class.

Rolling the dice

To get a random number x in the inclusive range of [1:6], instead of rand.nextInt(7) and skipping zeros in a while loop, you should use 1 + rand.nextInt(6) to get the same effect.

The roll6, roll10, roll20 methods all use the same logic. Just one roll method with a parameter could do the job:

public class Dice {
    private final Random randrandom = new Random();

    public int roll(int max) {
        return 1 + randrandom.nextInt(max);
    }
    
    public int roll6() {
        return roll(6);
    }
    
    // and so on ...
}

As demonstrated in this example, you probably don't need a new Random instance before each roll.

Chaining conditions

These conditions are mutually exclusive, therefore they should be chained with else if rather than using multiple independent if statements:

if(charclass.charAt(0) == 'w'){
    buildWarrior();
}
if(charclass.charAt(0) == 'a'){
    buildArcher();
}
if(charclass.charAt(0) == 'm'){
    buildMage();
}

Don't repeat yourself

There are near-duplicate lines at multiple places. For example the enemyattack and attack methods implement the same logic, with only minor differences in the details. Those details can be parameters, so that you can avoid copy-pasting code. Later when you need to update copy-pasted sections, it will be extremely annoying to make parallel changes at multiple locations, and it will be very error-prone too. Probably 99% of the time it's better to extract common logic and generalize than to copy-paste.

Formatting

You're using Eclipse Luna, it has a feature to reformat nicely the entire code. It's good to use that until it becomes a natural habit to type nicely.

OOP

This task really begs for object oriented programming.

As a first step, I suggest to move the maxhp, playerhp, playermeleedmg, xp, level fields to a dedicated class for players, and rewrite the build* methods. You could create a PlayerFactory with the build* methods that create and return warriors, archers, mages, enemies. All these different kind of characters could inherit from a common base Player class.

Rolling the dice

To get a random number x in the inclusive range of [1:6], instead of rand.nextInt(7) and skipping zeros in a while loop, you should use 1 + rand.nextInt(6) to get the same effect.

The roll6, roll10, roll20 methods all use the same logic. Just one roll method with a parameter could do the job:

public class Dice {
    Random rand = new Random();

    public int roll(int max) {
        return 1 + rand.nextInt(max);
    }
    
    public int roll6() {
        return roll(6);
    }
    
    // and so on ...
}

As demonstrated in this example, you probably don't need a new Random instance before each roll.

Chaining conditions

These conditions are mutually exclusive, therefore they should be chained with else if rather than using multiple independent if statements:

if(charclass.charAt(0) == 'w'){
    buildWarrior();
}
if(charclass.charAt(0) == 'a'){
    buildArcher();
}
if(charclass.charAt(0) == 'm'){
    buildMage();
}

Don't repeat yourself

There are near-duplicate lines at multiple places. For example the enemyattack and attack methods implement the same logic, with only minor differences in the details. Those details can be parameters, so that you can avoid copy-pasting code. Later when you need to update copy-pasted sections, it will be extremely annoying to make parallel changes at multiple locations, and it will be very error-prone too. Probably 99% of the time it's better to extract common logic and generalize than to copy-paste.

Formatting

You're using Eclipse Luna, it has a feature to reformat nicely the entire code. It's good to use that until it becomes a natural habit to type nicely.

OOP

This task really begs for object oriented programming.

As a first step, I suggest to move the maxhp, playerhp, playermeleedmg, xp, level fields to a dedicated class for players, and rewrite the build* methods. You could create a PlayerFactory with the build* methods that create and return warriors, archers, mages, enemies. All these different kind of characters could inherit from a common base Player class.

Rolling the dice

To get a random number x in the inclusive range of [1:6], instead of rand.nextInt(7) and skipping zeros in a while loop, you should use 1 + rand.nextInt(6) to get the same effect.

The roll6, roll10, roll20 methods all use the same logic. Just one roll method with a parameter could do the job:

public class Dice {
    private final Random random = new Random();

    public int roll(int max) {
        return 1 + random.nextInt(max);
    }
    
    public int roll6() {
        return roll(6);
    }
    
    // and so on ...
}

As demonstrated in this example, you probably don't need a new Random instance before each roll.

Chaining conditions

These conditions are mutually exclusive, therefore they should be chained with else if rather than using multiple independent if statements:

if(charclass.charAt(0) == 'w'){
    buildWarrior();
}
if(charclass.charAt(0) == 'a'){
    buildArcher();
}
if(charclass.charAt(0) == 'm'){
    buildMage();
}

Don't repeat yourself

There are near-duplicate lines at multiple places. For example the enemyattack and attack methods implement the same logic, with only minor differences in the details. Those details can be parameters, so that you can avoid copy-pasting code. Later when you need to update copy-pasted sections, it will be extremely annoying to make parallel changes at multiple locations, and it will be very error-prone too. Probably 99% of the time it's better to extract common logic and generalize than to copy-paste.

Formatting

You're using Eclipse Luna, it has a feature to reformat nicely the entire code. It's good to use that until it becomes a natural habit to type nicely.

Source Link
janos
  • 113.1k
  • 15
  • 154
  • 396
Loading