For the Main.battle()
you could refactor a lot:
- Magic numbers for the mana check conditions, recovery and costs in
Main.battle()
should be constants, improving readability and maintainability. - Magic numbers in various
case
statements would be more readable if you mapped them to constants, e.g., ATTACK=1, DEFEND=2, etc. - The
while
condition could be an extracted local boolean variableatLeastOneDragonAlive = x.hp > 0 && y.hp > 0;
dragonX
anddragonY
are more readable thanx
andy
printAbilityMenu()
is misleading. Its main function is to get the choice of the player.switch(chooseAbility())
makes more sense to me.Dragon
exposes its implementation details (int atk, def, hp, hpMax, mp, mpMax, exp, lvl;
). Not so great for encapsulation, but perhaps they'll never change and you can live with it. An alternative could be providing methods likeDragon.takeDamage(...)
andDragon.recover(...)
that encapsulate rules, making the simulator simpler. It all depends on what changes in the future: if different races have rules about recovering, then your simulator will have a bunch of cases to figure it out. Encapsulating it in each type of monster would simplify the simulator and allow easy changes to adding new monsters if the rules for recovery vary.
EDIT:
Builder the way you've done it is questionable here. According to my copy of the GoF, Builder's intent is
Separate the construction of a complex object from its representation so that the same construction process can create different representations.
What's the problem you're trying to solve with a Builder? To me, it means that in the future you'll be adding some new attributes that may not apply to some dragons, and you want a construction of a Dragon to be flexible. Or, dragons are composed of different things when they're born (that doesn't seem right to me as all dragons have all the attributes you show). A single constructor for your Dragons, even though it might have a bunch of arguments, is probably adequate. I did a combat simulator that had code like
hero1 = new Hero("Panos", 13, 11, Weapon.MORNINGSTAR, Armor.LEATHER, Shield.SMALL_SHIELD);
Builder is extra complexity compared to a single constructor. The construction process is maybe slightly more readable (my example with a single constructor doesn't show what 13 and 11 are, whereas with a builder it would bestrength(13)
anddexterity(11)
). I'm not sure that advantage is worth all the complexity of a Builder. See more controversy about Builder at http://stackoverflow.com/questions/1638722/how-to-improve-the-builder-pattern https://stackoverflow.com/questions/1638722/how-to-improve-the-builder-pattern -- that said, I think you implemented it fine.
For the Main.battle()
you could refactor a lot:
- Magic numbers for the mana check conditions, recovery and costs in
Main.battle()
should be constants, improving readability and maintainability. - Magic numbers in various
case
statements would be more readable if you mapped them to constants, e.g., ATTACK=1, DEFEND=2, etc. - The
while
condition could be an extracted local boolean variableatLeastOneDragonAlive = x.hp > 0 && y.hp > 0;
dragonX
anddragonY
are more readable thanx
andy
printAbilityMenu()
is misleading. Its main function is to get the choice of the player.switch(chooseAbility())
makes more sense to me.Dragon
exposes its implementation details (int atk, def, hp, hpMax, mp, mpMax, exp, lvl;
). Not so great for encapsulation, but perhaps they'll never change and you can live with it. An alternative could be providing methods likeDragon.takeDamage(...)
andDragon.recover(...)
that encapsulate rules, making the simulator simpler. It all depends on what changes in the future: if different races have rules about recovering, then your simulator will have a bunch of cases to figure it out. Encapsulating it in each type of monster would simplify the simulator and allow easy changes to adding new monsters if the rules for recovery vary.
EDIT:
Builder the way you've done it is questionable here. According to my copy of the GoF, Builder's intent is
Separate the construction of a complex object from its representation so that the same construction process can create different representations.
What's the problem you're trying to solve with a Builder? To me, it means that in the future you'll be adding some new attributes that may not apply to some dragons, and you want a construction of a Dragon to be flexible. Or, dragons are composed of different things when they're born (that doesn't seem right to me as all dragons have all the attributes you show). A single constructor for your Dragons, even though it might have a bunch of arguments, is probably adequate. I did a combat simulator that had code like
hero1 = new Hero("Panos", 13, 11, Weapon.MORNINGSTAR, Armor.LEATHER, Shield.SMALL_SHIELD);
Builder is extra complexity compared to a single constructor. The construction process is maybe slightly more readable (my example with a single constructor doesn't show what 13 and 11 are, whereas with a builder it would bestrength(13)
anddexterity(11)
). I'm not sure that advantage is worth all the complexity of a Builder. See more controversy about Builder at http://stackoverflow.com/questions/1638722/how-to-improve-the-builder-pattern -- that said, I think you implemented it fine.
For the Main.battle()
you could refactor a lot:
- Magic numbers for the mana check conditions, recovery and costs in
Main.battle()
should be constants, improving readability and maintainability. - Magic numbers in various
case
statements would be more readable if you mapped them to constants, e.g., ATTACK=1, DEFEND=2, etc. - The
while
condition could be an extracted local boolean variableatLeastOneDragonAlive = x.hp > 0 && y.hp > 0;
dragonX
anddragonY
are more readable thanx
andy
printAbilityMenu()
is misleading. Its main function is to get the choice of the player.switch(chooseAbility())
makes more sense to me.Dragon
exposes its implementation details (int atk, def, hp, hpMax, mp, mpMax, exp, lvl;
). Not so great for encapsulation, but perhaps they'll never change and you can live with it. An alternative could be providing methods likeDragon.takeDamage(...)
andDragon.recover(...)
that encapsulate rules, making the simulator simpler. It all depends on what changes in the future: if different races have rules about recovering, then your simulator will have a bunch of cases to figure it out. Encapsulating it in each type of monster would simplify the simulator and allow easy changes to adding new monsters if the rules for recovery vary.
EDIT:
Builder the way you've done it is questionable here. According to my copy of the GoF, Builder's intent is
Separate the construction of a complex object from its representation so that the same construction process can create different representations.
What's the problem you're trying to solve with a Builder? To me, it means that in the future you'll be adding some new attributes that may not apply to some dragons, and you want a construction of a Dragon to be flexible. Or, dragons are composed of different things when they're born (that doesn't seem right to me as all dragons have all the attributes you show). A single constructor for your Dragons, even though it might have a bunch of arguments, is probably adequate. I did a combat simulator that had code like
hero1 = new Hero("Panos", 13, 11, Weapon.MORNINGSTAR, Armor.LEATHER, Shield.SMALL_SHIELD);
Builder is extra complexity compared to a single constructor. The construction process is maybe slightly more readable (my example with a single constructor doesn't show what 13 and 11 are, whereas with a builder it would bestrength(13)
anddexterity(11)
). I'm not sure that advantage is worth all the complexity of a Builder. See more controversy about Builder at https://stackoverflow.com/questions/1638722/how-to-improve-the-builder-pattern -- that said, I think you implemented it fine.
For the Main.battle()
you could refactor a lot:
- Magic numbers for the mana check conditions, recovery and costs in
Main.battle()
should be constants, improving readability and maintainability. - Magic numbers in various
case
statements would be more readable if you mapped them to constants, e.g., ATTACK=1, DEFEND=2, etc. - The
while
condition could be an extracted local boolean variableatLeastOneDragonAlive = x.hp > 0 && y.hp > 0;
dragonX
anddragonY
are more readable thanx
andy
printAbilityMenu()
is misleading. Its main function is to get the choice of the player.switch(chooseAbility())
makes more sense to me.Dragon
exposes its implementation details (int atk, def, hp, hpMax, mp, mpMax, exp, lvl;
). Not so great for encapsulation, but perhaps they'll never change and you can live with it. An alternative could be providing methods likeDragon.takeDamage(...)
andDragon.recover(...)
that encapsulate rules, making the simulator simpler. It all depends on what changes in the future: if different races have rules about recovering, then your simulator will have a bunch of cases to figure it out. Encapsulating it in each type of monster would simplify the simulator and allow easy changes to adding new monsters if the rules for recovery vary.
EDIT:
Builder the way you've done it is questionable here. According to my copy of the GoF, Builder's intent is
Separate the construction of a complex object from its representation so that the same construction process can create different representations.
What's the problem you're trying to solve with a Builder? To me, it means that in the future you'll be adding some new attributes that may or may not apply to some dragons, and you want a construction of a Dragon to be flexible. Or, dragons are composed of different things when they're born (that doesn't seem right to me as all dragons have all the attributes you show). A single constructor for your Dragons, even though it might have a bunch of arguments, is probably adequate. I did a combat simulator that had code like
hero1 = new Hero("Panos", 13, 11, Weapon.MORNINGSTAR, Armor.LEATHER, Shield.SMALL_SHIELD);
Builder is extra complexity compared to a single constructor. The construction process is maybe slightly more readable (my example with a single constructor doesn't show what 13 and 11 are, whereas with a builder it would bestrength(13)
anddexterity(11)
). I'm not sure that advantage is worth all the complexity of a Builder. ThatSee more controversy about Builder at http://stackoverflow.com/questions/1638722/how-to-improve-the-builder-pattern -- that said, I think you implemented it fine.
For the Main.battle()
you could refactor a lot:
- Magic numbers for the mana check conditions, recovery and costs in
Main.battle()
should be constants, improving readability and maintainability. - Magic numbers in various
case
statements would be more readable if you mapped them to constants, e.g., ATTACK=1, DEFEND=2, etc. - The
while
condition could be an extracted local boolean variableatLeastOneDragonAlive = x.hp > 0 && y.hp > 0;
dragonX
anddragonY
are more readable thanx
andy
printAbilityMenu()
is misleading. Its main function is to get the choice of the player.switch(chooseAbility())
makes more sense to me.Dragon
exposes its implementation details (int atk, def, hp, hpMax, mp, mpMax, exp, lvl;
). Not so great for encapsulation, but perhaps they'll never change and you can live with it. An alternative could be providing methods likeDragon.takeDamage(...)
andDragon.recover(...)
that encapsulate rules, making the simulator simpler. It all depends on what changes in the future: if different races have rules about recovering, then your simulator will have a bunch of cases to figure it out. Encapsulating it in each type of monster would simplify the simulator and allow easy changes to adding new monsters if the rules for recovery vary.
EDIT:
Builder the way you've done it is questionable here. According to my copy of the GoF, Builder's intent is
Separate the construction of a complex object from its representation so that the same construction process can create different representations.
What's the problem you're trying to solve with a Builder? To me, it means that in the future you'll be adding some new attributes that may or may not apply to some dragons, and you want a construction of a Dragon to be flexible. Or, dragons are composed of different things when they're born (that doesn't seem right to me as all dragons have all the attributes you show). A single constructor for your Dragons, even though it might have a bunch of arguments, is probably adequate. I did a combat simulator that had code like
hero1 = new Hero("Panos", 13, 11, Weapon.MORNINGSTAR, Armor.LEATHER, Shield.SMALL_SHIELD);
Builder is extra complexity compared to a single constructor. The construction process is maybe slightly more readable (my example with a single constructor doesn't show what 13 and 11 are, whereas with a builder it would bestrength(13)
anddexterity(11)
). I'm not sure that advantage is worth all the complexity of a Builder. That said, I think you implemented it fine.
For the Main.battle()
you could refactor a lot:
- Magic numbers for the mana check conditions, recovery and costs in
Main.battle()
should be constants, improving readability and maintainability. - Magic numbers in various
case
statements would be more readable if you mapped them to constants, e.g., ATTACK=1, DEFEND=2, etc. - The
while
condition could be an extracted local boolean variableatLeastOneDragonAlive = x.hp > 0 && y.hp > 0;
dragonX
anddragonY
are more readable thanx
andy
printAbilityMenu()
is misleading. Its main function is to get the choice of the player.switch(chooseAbility())
makes more sense to me.Dragon
exposes its implementation details (int atk, def, hp, hpMax, mp, mpMax, exp, lvl;
). Not so great for encapsulation, but perhaps they'll never change and you can live with it. An alternative could be providing methods likeDragon.takeDamage(...)
andDragon.recover(...)
that encapsulate rules, making the simulator simpler. It all depends on what changes in the future: if different races have rules about recovering, then your simulator will have a bunch of cases to figure it out. Encapsulating it in each type of monster would simplify the simulator and allow easy changes to adding new monsters if the rules for recovery vary.
EDIT:
Builder the way you've done it is questionable here. According to my copy of the GoF, Builder's intent is
Separate the construction of a complex object from its representation so that the same construction process can create different representations.
What's the problem you're trying to solve with a Builder? To me, it means that in the future you'll be adding some new attributes that may not apply to some dragons, and you want a construction of a Dragon to be flexible. Or, dragons are composed of different things when they're born (that doesn't seem right to me as all dragons have all the attributes you show). A single constructor for your Dragons, even though it might have a bunch of arguments, is probably adequate. I did a combat simulator that had code like
hero1 = new Hero("Panos", 13, 11, Weapon.MORNINGSTAR, Armor.LEATHER, Shield.SMALL_SHIELD);
Builder is extra complexity compared to a single constructor. The construction process is maybe slightly more readable (my example with a single constructor doesn't show what 13 and 11 are, whereas with a builder it would bestrength(13)
anddexterity(11)
). I'm not sure that advantage is worth all the complexity of a Builder. See more controversy about Builder at http://stackoverflow.com/questions/1638722/how-to-improve-the-builder-pattern -- that said, I think you implemented it fine.
For the Main.battle()
you could refactor a lot:
- Magic numbers for the mana check conditions, recovery and costs in
Main.battle()
should be constants, improving readability and maintainability. - Magic numbers in various
case
statements would be more readable if you mapped them to constants, e.g., ATTACK=1, DEFEND=2, etc. - The
while
condition could be an extracted local boolean variableatLeastOneDragonAlive = x.hp > 0 && y.hp > 0;
dragonX
anddragonY
are more readable thanx
andy
printAbilityMenu()
is misleading. Its main function is to get the choice of the player.switch(chooseAbility())
makes more sense to me.Dragon
exposes its implementation details (int atk, def, hp, hpMax, mp, mpMax, exp, lvl;
). Not so great for encapsulation, but perhaps they'll never change and you can live with it. An alternative could be providing methods likeDragon.takeDamage(...)
andDragon.recover(...)
that encapsulate rules, making the simulator simpler. It all depends on what changes in the future: if different races have rules about recovering, then your simulator will have a bunch of cases to figure it out. Encapsulating it in each type of monster would simplify the simulator and allow easy changes to adding new monsters if the rules for recovery vary.
EDIT:
Builder the way you've done it is questionable here. According to my copy of the GoF, Builder's intent is
Separate the construction of a complex object from its representation so that the same construction process can create different representations.
What's the problem you're trying to solve with a Builder? To me, it means that in the future you'll be adding some new attributes that may or may not apply to some dragons, and you want a construction of a Dragon to be flexible. Or, dragons are composed of different things when they're born (that doesn't seem right to me as all dragons have all the attributes you show). A single constructor for your Dragons, even though it might have a bunch of arguments, is probably adequate. I did a combat simulator that had code like
hero1 = new Hero("Panos", 13, 11, Weapon.MORNINGSTAR, Armor.LEATHER, Shield.SMALL_SHIELD);
Builder is extra complexity compared to a single constructor. The construction process is maybe slightly more readable (my example with a single constructor doesn't show what 13 and 11 are, whereas with a builder it would bestrength(13)
anddexterity(11)
). I'm not sure that advantage is worth all the complexity of a Builder. That said, I think you implemented it fine.
For the Main.battle()
you could refactor a lot:
- Magic numbers for the mana check conditions, recovery and costs in
Main.battle()
should be constants, improving readability and maintainability. - Magic numbers in various
case
statements would be more readable if you mapped them to constants, e.g., ATTACK=1, DEFEND=2, etc. - The
while
condition could be an extracted local boolean variableatLeastOneDragonAlive = x.hp > 0 && y.hp > 0;
dragonX
anddragonY
are more readable thanx
andy
printAbilityMenu()
is misleading. Its main function is to get the choice of the player.switch(chooseAbility())
makes more sense to me.Dragon
exposes its implementation details (int atk, def, hp, hpMax, mp, mpMax, exp, lvl;
). Not so great for encapsulation, but perhaps they'll never change and you can live with it. An alternative could be providing methods likeDragon.takeDamage(...)
andDragon.recover(...)
that encapsulate rules, making the simulator simpler. It all depends on what changes in the future: if different races have rules about recovering, then your simulator will have a bunch of cases to figure it out. Encapsulating it in each type of monster would simplify the simulator and allow easy changes to adding new monsters if the rules for recovery vary.
For the Main.battle()
you could refactor a lot:
- Magic numbers for the mana check conditions, recovery and costs in
Main.battle()
should be constants, improving readability and maintainability. - Magic numbers in various
case
statements would be more readable if you mapped them to constants, e.g., ATTACK=1, DEFEND=2, etc. - The
while
condition could be an extracted local boolean variableatLeastOneDragonAlive = x.hp > 0 && y.hp > 0;
dragonX
anddragonY
are more readable thanx
andy
printAbilityMenu()
is misleading. Its main function is to get the choice of the player.switch(chooseAbility())
makes more sense to me.Dragon
exposes its implementation details (int atk, def, hp, hpMax, mp, mpMax, exp, lvl;
). Not so great for encapsulation, but perhaps they'll never change and you can live with it. An alternative could be providing methods likeDragon.takeDamage(...)
andDragon.recover(...)
that encapsulate rules, making the simulator simpler. It all depends on what changes in the future: if different races have rules about recovering, then your simulator will have a bunch of cases to figure it out. Encapsulating it in each type of monster would simplify the simulator and allow easy changes to adding new monsters if the rules for recovery vary.
EDIT:
Builder the way you've done it is questionable here. According to my copy of the GoF, Builder's intent is
Separate the construction of a complex object from its representation so that the same construction process can create different representations.
What's the problem you're trying to solve with a Builder? To me, it means that in the future you'll be adding some new attributes that may or may not apply to some dragons, and you want a construction of a Dragon to be flexible. Or, dragons are composed of different things when they're born (that doesn't seem right to me as all dragons have all the attributes you show). A single constructor for your Dragons, even though it might have a bunch of arguments, is probably adequate. I did a combat simulator that had code like
hero1 = new Hero("Panos", 13, 11, Weapon.MORNINGSTAR, Armor.LEATHER, Shield.SMALL_SHIELD);
Builder is extra complexity compared to a single constructor. The construction process is maybe slightly more readable (my example with a single constructor doesn't show what 13 and 11 are, whereas with a builder it would bestrength(13)
anddexterity(11)
). I'm not sure that advantage is worth all the complexity of a Builder. That said, I think you implemented it fine.