OO Design & avoiding getters/setters

[Jared Christian]

I consider myself past the beginners stage but not quite ready for a full time gig. My goal is to become a developer, but since I am pretty much completely self-taught, I'm having trouble going from that intermediate level to a point where an employer would feel confident in hiring me on even as a Junior Dev.

Either way, I determined that having some sort of a portfolio would be required. With that decided, I embarked on creating a program that is based on my love and enjoyment of pen & paper roleplaying games. I'm creating an application that will make it easier for the storyteller of the game to create encounters for the players and so forth since the game is pretty math-heavy and complicated. This made me think it would be perfect for reinforcing programming concepts. I am gearing this to end up with some sort of UI, whether it be JavaFX or even turned into an Android app. I would like my portfolio projects to showcase my understanding of OO design.

That said, I'm having trouble designing classes after learning that I should probably avoid using getters/setters (see http://www.javaworld.com/article/2073723/core-java/why-getter-and-setter-methods-are-evil.html). I have a Character class used to represent a character in the game. This class has several fields: Ability attributes such as Strength, Dexterity, etc, the character's chosen character class (Fighter, Barbarian, Wizard, etc...), the character's skills which have allocated points as selected by the player, equipment and items, and basic fields like how many health points they have for example. I would also like to grant the ability of the user to save data.

The problem I have is trying to figure out how I can avoid using getters/setters while also allowing for the changes that will happen as the game goes on. Eventually the character will level up after gaining X number of experience, so they might have improvements to one of their ability scores (Strength for example). They may pick up a magic item that temporarily improves a score so long as they have it in their possession (not permanent). I've read that you shouldn't sit down with a preconceived class to build around, but Character seems to make the most sense for this since just about all of its internal variables will change at some point and they need to be kept track of. I'm not sure how else to go about doing that, but that might simply be my ignorance.

Here's basically what I have so far:

public class Character {
private String name;
private Race race;
private CharacterClass charClass;
private STR strength;
...(more attributes)
private HashMap<String,ArrayList> skills; //Maps the skill name to a list of modifiers which make up the total bonus since these should be visible for reference.
}

//Constructors go here... Depending on if the user wants to select a class or race immediately, they might call the methods below...

	public void setRace(String raceName){
		RaceFactory rf = new RaceFactory();
		this.race = rf.getRace(raceName);
	}

	public void setClass(String name, int levels){
		ClassFactory cf = new ClassFactory();
		charClass = cf.getClass(name);
		charClass.setLevels(levels);
	}

I am using the Factory pattern in order for the user to select which character class or race (Dwarf, Elf, etc...) for their character. Skills will be updated as the character levels, and ability scores (STR, etc...) can be altered and modified. Based on the character's chosen race, their ability scores are modified, though that is a permanent adjustment. Some aspects of a character should be changeable, but others shouldn't. They won't be able to change race without creating an entirely new character, for example. But, initially that should be something that can be changed as the character is being created.

I'm trying to design the class so that all of the field objects (STR, skills, charClass, race, etc...) are doing the work and keeping track of their own state. Is the above good OO design? Originally I had the Character class doing and keeping track of everything and after more experience/knowledge gathering, I made the non-primitives objects of their own.

Maybe I really need to brush up on my thinking as it relates to OO because in my mind, I'm having a hard time balancing immutable classes and encapsulation with changeable fields or objects. Do you just set it up to create an entirely new object each time you need to alter a field, such as if a Character picks up a magic belt which boosts his Strength score, you create an entirely new Strength object to assign to the Character object, or create an entirely new Character object with that one change to the Strength object? I have a good feeling that's not the case, but I still have difficulty with the idea.

I probably need to do some reading, but I find I learn best by doing, and so I was looking for a little guidance as it relates to my current project. I am definitely all ears when it comes to book suggestions though. Any guidance or pointers anyone can offer would be greatly appreciated.

[Stephan van Hulst]

Hi Jared, welcome to CodeRanch!

Getters and setters are not evil. Without them, we wouldn't be able to do anything. I think the article (I didn't really read it) is mostly concerned with the fact that people blindly add them to a class, without thinking about what responsibilities the class really has. They provide accessors for every field, even if that exposes too much information.

Think about what you want to know about instances of your types. Think about what you want to be able to do to instances of your types. Make methods appropriate to those requirements.

Before we continue, I want to point out that you gave your class the same name as an important class in the java.lang package. This is a very bad idea. Don't name it Character, name it something like Creature instead. From this point on I'm going to refer to your type as Creature. I'm also going to use Job instead of Class, because of all the annoying identifier problems you're going to encounter with the class keyword.

I imaging you would like to be able to retrieve information from a creature such as its name, its race, its class. It's fine to expose these things through getters. Personally, I'm not a big fan of the whole "getFoo()" theme; if my type contains 'properties' that don't require computation to retrieve, I prefer to just call the accessor after the field:

final class Creature {

  String name() {}

  Race race() {}

  ...

}

Not all types should be immutable. If a type isn't used as a 'value', it's perfectly reasonable to give it setters. A creature is a fluid thing, it's to be expected that it changes over time. Don't make everything mutable, but I imagine that some of its attributes such as its strength may change over time. It's fine to give Creature a setStrength() method.

Another thing, don't over-engineer things that really can remain simple. If STR is nothing but a counter which you can increase or decrease, just use an int, and make the Creature class responsible for keeping the values within a valid range. If however all attributes have common behavior, you could make an Attribute class for things such as strength, intelligence, etc. and make it responsible for range checking.

Note that because a lot of these types only apply to fields of the Creature class, you could make them nested types, such as Creature.Race, Creature.Job, Creature.Attribute

You're not using the Factory pattern correctly. The idea of the factory is that you pass it to types so they can later use it to create instances of a different type. It makes no sense to create a factory in the same method you use it. Also don't pass weak types such as strings to setters and constructors. Use strong types directly:

final class Creature {

  Creature(String name, Race race, Job job) {
    //
  }

  String name() {
    //
  }

  Race race() {
    //
  }

  void setRace(Race race) {  // are you really sure you can change a creature's race?
    //
  }

  Job job() {
    //
  }

  void setJob(Job job) {
    //
  }
}

While a Creature isn't immutable, something like a Race most certainly is. If you have a reason not to make a Race an enum, you should definitely make it immutable in another way.

[Stephan van Hulst]

Another comment: HashMap<String,ArrayList> skills is raw and weakly typed. What does the String stand for? What does the list hold? Why is it an ArrayList and not a List? To me, a Set<Skill> makes more sense.

[Dennis Grimbergen]

Welcome. High quality first post (give this guy a cow for that?)

I want to tell you a little about how I became a java developer with a job. Maybe you find it useful.

I finished university in 2001 in biophysical chemistry. After that I worked 4.5 years as a PhD. During that time I started with java in my free-time as a hobby. Reading some tutorials and trying some small examples with a simple java IDE. During that time I liked to chat. Around the year 2000 java chat applets were all around. I wanted my own chat. And I wanted it that same day. Ofcourse because of my lack of skills it wasn't a success. So I kept on reading...always trying to use the code conventions and stuff. It took me a small year to be the proud owner of a working chat system. And then I thought...this can be better...so I wrote a second version of the chatbox (eventually I wrote 5 versions in the end). It was fun and useful.

In 2006 my work in the laboratory ended as a PhD. Then I realized that I wanted to be a programmer, not a scientist. I applied for a junior java developer job. And I had print outs of my cool chat applets and an online stratego applet I had made. A picture says more than a thousand words. I got a contract for 6 months (at a very low salary, ±850 euro / 950 dollar, lower than what I had). But hey...I got a job as a developer. If you really want a change, it is an investment in your future....it really is.

I understand that you call yourself an 'intermediate' level. That probably applies to your knowledge and skills using JavaSE. I had the same, and it was not correct. When you start with a job, you probably end up in a team of people, do all kinds of projects the same time. This project based working, like Agile/Scrum, or DevOps techniques (please look them up so you know what they mean (because you need that knowledge)), takes some time to get familiar with. When you start, you might feel a junior (which you more or less are at such moment).

If you think you have a decent knowledge of java and you are good at analyzing things in general, and you are decent in math, then you could be a good developer. Let your heart be your guide, because in the end you truly know if you are far enough to start a carreer in programming. A little hesitation is ok, it doesn't mean you aren't ready for it!)

I would hire you as a junior, because you show more passion than average.

[Jared Christian]

Stephan, thanks for your time in responding. I really appreciate it.

You're right about my class naming... shame on me. There are so many conflicting names between this game system and Java. INT is the abbreviation for intelligence, class is the term for a character's 'job' as you put it. Haha. I guess I will have to be more creative in coming up with substitute names to avoid clashing with keywords and whatnot.

In regards to the STR attribute, there is a basic int value for it, but it has an associated modifier. A Strength of 14 for example, has a +2 modifier. You don't reach a +3 until your Strength becomes 16. Something needs to calculate the modifier and keep track of temporary boosts to the attribute (such as magic) and then return it to the normal base score later. The modifier is used as a bonus for attacks, damage, and also skills. That said, it still may be a good idea to have nested classes such as Creature.Race since players are only able to select from a limited list of races whereas monsters have a lot more options. At the same time, monsters should have access to the standard races as well. Monsters do have a Type (Undead, Plant, Monstrous Humanoid, for example) that they use, but you can also build a Monster as one of the core races.

Some of these fields will also apply to other classes as well, such as the Monster class. Monsters have their own attributes (STR, DEX, etc...) and typically you have a group of four players (Creatures) who go up against Monsters in groups of varying sizes to battle one another. Monsters can be customized and have player class (Wizard, Fighter) added to them, but they should not have access to certain things. It might have to be it's own class even though it has a lot of the same attributes. I don't think subclassing them will work, but it may be possible.

As far as the HashMap, the String is the name of the Skill. (Acrobatics, Bluff, Diplomacy, etc...). It's mainly a placeholder for now so I don't have to look at errors in Eclipse telling me there is no "Skill" class yet. Skills have to keep track of all the modifiers involved and need to have methods to roll (dice) for those skills and then add their bonus. For an example, a Creature might have 3 skill ranks in Swim, a STR of 15 (+2) and a Ring of Swimming (grants a +5). All of these separate modifiers add up to become a net +10 to their Swim checks. I think a Set does make a lot more sense. There are no duplicate skill names, and the skill object itself will hold all of the individual modifiers which make up the bonus... it just needs to communicate with the ability scores, magic items and so on to know what those modifiers are at all times. If any of them change, it needs to be updated. I definitely see that as a better option.

My thinking in going with the Factory was I am going to have a pop-up appear when a user decides to create a new Creature. Think File > New > Creature and then you get a creation wizard. I would like the program to be able to change race on the wizard to see how it affects their ability scores, but the program doesn't know which race to create until the user chooses it. A Dwarf, for example, would boost Strength by 2, but there are also two Dwarf sub-races such as Hill Dwarf or Mountain Dwarf that boosts other scores in addition. If the user decides they would rather be Human, then it would need to adjust the scores again. My thoughts behind my so-called Factory was taking a String variable and then translate that into a concrete object since the application doesn't know what race or class the user is going to select at runtime. I guess I just need to find a good way to do that or understand how best to go about doing that. Now that I'm thinking about it, I've been coding this almost as if the user is going to be typing in what race they want and then have the program translate that into a concrete object which doesn't make sense from any kind of a reasonable UI perspective.

[Jared Christian]

Dennis,

Thanks very much for your comments. It was really encouraging to hear you say you would hire me as a junior. Right now I'm in IT but a different area... desktop/server systems support. I was working towards becoming a server admin by going to school for network administration when I had to take a JavaScript course. I realized how much more I enjoyed programming and decided to switch gears. I'm still really intimidated by what I recognize as a skills gap between my current knowledge and the level of knowledge/experience/understanding that professional developers have. I have the drive to get to that point but I don't know the best route to take aside from simply trying to develop things and then have others critique. Perhaps that's the answer. Sometimes I feel like someone showed me how to drive a stick-shift and then handed me a book on advanced engine engineering after I told them I wanted to learn how to work on engines. Not the greatest analogy, but it's how I feel at times. There are a lot of good resources on Java but it seems to go from 0 to 60. Here's the basics, and then here's the Ph.D level material. Then I have trouble determining which framework to learn. Should I just pick one, or should I just stick with SE until I've created several programs? Not sure if I want to go the Android route or Web and the Spring framework for example. I wish there was a general roadmap to provide guidance, such as learn x, y, and z, then pick a framework, learn it, and then look for a job.

[Stephan van Hulst]

I have had quite some experience with naming conflicts myself, and for some weird reason it seems especially bad with RPG type problem domains. Don't worry, you'll find something that works.

I wouldn't make a dedicated class to handle the counter per attribute. Instead, make a general Attribute enum, and simply let a creature have a Map<Attribute, Integer> that keeps track of the base attribute scores. You can do the same for Skill, where you have a map from an enum to the number of base skill points. You can associate each skill with an attribute (or multiple), and then the Creature is responsible for applying modifiers to the skill, when you need to calculate the total skill score. It will also factor in effects bestowed by items. You can just calculate this on the fly when the skill is requested through a method.

I don't even think subclassing for monsters is necessary. Players and monsters are both creatures. If you want to restrict what classes, races or types they can use, you can do this through data instead of through code.

You don't need factories for character creation. Just make a creature instance, give it some default properties, and then through setters, your character editor can just modify the creature directly.

[Stephan van Hulst]

Dennis, I believe Ranch Hands are able to give a cow a day themselves

[Dennis Grimbergen]

Stephan van Hulst wrote:Dennis, I believe Ranch Hands are able to give a cow a day themselves

I didn't know that, but I read it somewhere on this site after you told me.
But I still have no idea how to give a cow?

[Paul Clapham]

If you do indeed have the right to give a cow, then there should be a little "cow" icon in the post which you want to give a cow to. For me it's just to the right of the "Report!" icon and when I hover over it the popup says "give one Cow".

[Tyson Lindner]

I actually think the reasoning to avoid getters and setters somewhat applies here. Well, not for getters. I have no problem with every attribute in your CharacterClass class to be paired with a simple getter. With setters on the other hand its a different matter. We don't really want to be using simple code to just "set" our attributes to a certain level. We should have rather complicated methods to do so that take other game states into account. For example, say we take damage from a goblin. That goblin might have an attack power of 3, and if our life is 10, we might be tempted to call a method setLife(7), but the code for that logic has to actually go somewhere and its possible we can have a lot more variables. For example we might have leather armor that reduces attack damage by 1, and a shield ward that also reduces it by 1. Its much better to have and call an internal method takeDamage(3) that takes into account our armor, wards, magic amulets, etc. that correctly calculates and increments our life loss/gain (1 in this case). Another example would be leveling up. Instead of calling setLevel(8), we should be calling gainXP(*some value*) which may call levelUp(), the latter method being a complicated one that increments all our attributes according to some algorithm. The point is that our character attributes shouldn't be randomly set or incremented throughout the game. We should have events that happen which are handled by our character class which then sets the values internally according to some methodology.

[Campbell Ritchie]

There are hazards with getXXX methods. If you simply return a reference to a mutable reference type, you can lose control over the state of that object. Search for “defensive copy/ies/ing.”