views:

458

answers:

10

Let's say I am tasked with coding some kind of an RPG. This means that, for example, I'll want to track a Character GameCharacter and stats thereof, like intelligence, damage bonuses or hitpoints.

I'm positively scared that by the end of the project I may end up with handling with very a high number of fields - and for each I would have to make sure they follow a very similar set of constraint and behaviours (for example, I want them to be bounded between a min and a max; I want to be able to distinguish between a "base value" and a "temporary bonus"; I want to be able to increment and decrement both without going through a setters and getters). Suddenly, for every field I would need one (two?) getter and four setters and maybe a couple resetters too! Even for 10 fields that means a LOT of methods all alike, eek.

For DRYness I have started encapsulating the logic of messing with those stats in Field classes, so that I could write code such as intelligence.applyBonus(10) or hitpoints.get() (which takes care the value returned is in range), etc. I have even gone to such a length to create classes to group those fields together, but that's not the point right now.

Now, I hit in this problem while "plugging" Field into GameCharacter: most Java textbooks say that each class should have private fields with public getters and setters. That sounds good in theory, and I've already built a whole class around an int; however, the idea doesn't sound as solid when you find yourself calling a getter to get... a getter:

thisCharacter.getIntelligence().get() //eeek

I'd much rather access the field directly. Maybe it's my Python/VB [1] "background", but for me it is cleaner, clearer and more straightforward:

thisCharacter.intelligence.get()

The (theoretical) problem with public fields is that I'm giving up all control on it; for example at some other point in the codebase, by misfortune, the following might happen:

thisCharacter.intelligence = somethingThatReallyIsNull;

Sounds like a subtle bug... but... I mean, should I really worry about that? I for one never plan to assign the Field directly [2], I have documented in the Javadoc that this is not something that should be done, but still I am new here so I'm a bit torn.

So I would like to hear what your take on this topic. Are the advantages of encapsulation so massive that I should go ahead and have getter getters and setter getters and so on... or should I take encapsulation in healthy measures and leave the Field as a public field?


[1] Yes, I know. I've been trying to forget. But we've just recently seen also a bit of C# and man, aren't properties sweet. Oh well.

[2] except in Constructors! And a getter won't save me from a faulty constructor.

+7  A: 

My experience is that in situations where you need a lot of fields, the number, nature, naming, and types of fields are so flexible and likely to change throughout the lifetime of your project that you would likely need some sort of map instead of fields.

For example have an attribute map from keys to values.

Provide public calls for getting and setting the attributes, but don't let everybody use them (or make sure they don't). Instead, create classes to represent each attribute you are interested in, and that class provides all the functions for manipulating that attribute. For example, if you have Strength, you could have a "StrengthManipulation" class that is initialized to a specific Player object, and then provides getters, setters (All with appropriate validation and exceptions), and perhaps things like calculating strength with bonuses, etc.

One advantage of this is that you decouple the use of your attributes from your player class. So if you now add an Intelligence attribute, you don't have to deal and recompile everything that manipulates only strength.

As for accessing fields directly, it's a bad idea. When you access a field in VB (at least in old VBs), you usually call a property getter and setter and VB simply hides the () call for you. My view is that you have to adapt to the conventions of the language that you are using. In C, C++, Java and the like you have fields and you have methods. Calling a method should always have the () to make it clear that it is a call and other things may happen (e.g., you could get an exception). Either way, one of the benefits of Java is its more precise syntax and style.

VB to Java or C++ is like Texting to graduate school scientific writing.

BTW: Some usability research shows that it's better to not have parameters to constructors and rather construct and call all the setters if you need them.

Uri
I agree with Uri. Having a map is a lot easier to maintain and scale, rather than using fields.
Jose Chavez
cf, The Universal Design Pattern: http://steve-yegge.blogspot.com/2008/10/universal-design-pattern.html
jsight
I always thought of it as a fairly straightforward idiom, but I guess anything straightforward ends up a pattern :)
Uri
+2  A: 

To me it looks like 'thisCharacter' might well have an 'intelligence' object for dealing with intelligence behind the scenes, but I question whether that should be public at all. You should just expose thisCharacter.applyInt and thisCharacter.getInt instead of the object that deals with it. Dont expose your implementation like that.

willcodejavaforfood
+1  A: 

Keep your fields private! You never want to expose too much of your API. You can always make something that was private public, but not the other way round, in future releases.

Think as if you'll make that into the next MMORPG. You'd have a lot of scope for bugs, errors, and unnecessary evil. Ensure immutable properties are final.

Think of a DVD player, with its miniamilistic interface (play, stop, menu), and yet such technicality inside. You'll want to hide everything non-vital in your program.

Beau Martínez
+1  A: 

It sounds like your main complaint isn't so much with the abstraction of setter/getter methods, but with the language syntax for using them. Ie, you'd prefer something like C# style properties.

If this is the case, then the Java language has relatively little to offer you. Direct field access is fine, until you need to switch to a getter or setter, and then you will have some refactoring to do (possibly ok, if you control the whole codebase).

Of course, if the Java platform is a requirement, but the language isn't then there are other alternatives. Scala has a very nice property syntax, for example, along with lots of other features that could be useful for such a project. And best of all, it runs on the JVM, so you still get the same portability that you'd get by writing it in the Java language. :)

jsight
+3  A: 

Steve Yegge had a very interesting (if lengthy) blog post that covered these issues: The Universal Design Pattern.

John Topley
Good call! That's a great article in defense of the "properties" pattern.
erickson
+1  A: 

What you appear to have here is a single layer of a composite model.

You might want to add methods that add abstractions to the model, rather than just having it as a set of lower level models.

The fields should be final, so even if you did make them public you couldn't accidentally assign null to them.

The "get" prefix is present for all getters, so it's probably more the initial look than a new problem as such.

Tom Hawtin - tackline
A: 

In addition to Uri's answer (which I totally support), I'd like to suggest you consider defining the attribute map in data. This will make your program EXTREMELY flexible and will factor out a lot of code you don't even realize you don't need.

For instance, an attribute can know what field it binds to on the screen, what field it binds to in the database, a list of actions to execute when the attribute changes (a recalculate hit% might apply to both strength and dex...)

Doing it this way, you have NO code per attribute to write a class out to the DB or display it on the screen. You simply iterate over the attributes and use the information stored inside.

The same thing can apply to skills--in fact, attributes and skills would probably derive from the same base class.

After you go down this road, you'll probably find yourself with a pretty serious text file to define attributes and skills, but adding a new skill would be as easy as:

Skill: Weaving
  DBName: BasketWeavingSkill
  DisplayLocation: 102, 20  #using coordinates probably isn't the best idea.
  Requires: Int=8
  Requires: Dex=12
  MaxLevel=50

At some point, adding a skill like this would require no code change whatsoever, it could all be done in data pretty easily, and ALL the data is stored in a single skill object attached to a class. You could, of course, define actions the same way:

Action: Weave Basket
  text: You attempt to weave a basket from straw
  materials: Straw
  case Weaving < 1
    test: You don't have the skill!
  case Weaving < 10
    text: You make a lame basket
    subtract 10 straw
    create basket value 8
    improve skill weaving 1%
  case Weaving < 40
    text: You make a decent basket
    subtract 10 straw
    create basket value 30
    improve skill weaving 0.1%
  case Weaving < 50
    text: You make an awesome basket!
    subtract 10 straw
    create basket value 100
    improve skill weaving 0.01%
  case Weaving = 50
    text: OMG, you made the basket of the gods!
    subtract 10 straw
    create basket value 1000

Although this example is pretty advanced, you should be able to visualize how it would be done with absolutely no code. Imagine how difficult it would be to do anything like that without code if your "attributes/skills" were actually member variables.

Bill K
+10  A: 

Sounds like you're thinking in terms of if(player.dexterity > monster.dexterity) attacker = player. You need to think more like if(player.quickerThan(monster)) monster.suffersAttackFrom(player.getCurrentWeapon()). Don't mess around with bare stats, express your actual intention and then design your classes around what they're supposed to do. Stats are a cop-out anyway; what you really care about is whether a player can or cannot do some action, or their ability/capacity versus some reference. Think in terms of "is the player character strong enough" (player.canOpen(trapDoor)) instead of "does the character have at least 50 strength".

TMN
erickson
While you do bring an excellent point, I am afraid it's fair to reconsider Uri's as the "answer" of the thread. :)
badp
+1  A: 

Are the advantages of encapsulation so massive that I should go ahead and have getter getters and setter getters and so on... or should I take encapsulation in healthy measures and leave the Field as a public field?

IMO, encapsulation has nothing to do with wrapping a getter/setter around a private field. In small doses, or when writing general-purpose libraries, the tradeoff is acceptable. But when left unchecked in a system like the one you're describing, its a an antipattern.

The problem with getters/setters is that they create overly tight coupling between the object with those methods and the rest of the system.

One of the advantages of real encapsulation is that it reduces the need for getters and setters, decoupling your object from the rest of the system in the process.

Rather than exposing the implementation of GameCharacter with setIntelligence, why not give GameCharacter an interface that better reflects it's role in the game system?

For example, instead of:

// pseudo-encapsulation anti-pattern
public class GameCharacter
{
  private Intelligence intelligence;

  public Intelligence getIntelligence()
  {
    return intelligence
  }

  public void setIntelligence(Intelligence intelligence)
  {
    this.intelligence = intelligence;
  }
}

why not try this?:

// better encapsulation
public class GameCharacter
{
  public void grabObject(GameObject object)
  {
    // TODO update intelligence, etc.
  }

  public int getIntelligence()
  {
    // TODO
  }
}

or even better:

// still better
public interface GameCharacter
{
  public void grabObject(GameObject object); // might update intelligence
  public int getIntelligence();
}

public class Ogre implements GameCharacter
{
  // TODO: never increases intelligence after grabbing objects
}

In other word, a GameCharacter can grab GameObjects. The effect of each GameCharacter grabbing the same GameObject can (and should) vary, but the details are fully encapsulated within each GameCharacter implementation.

Notice how the GameCharacter is now in charge of handling its own intelligence update (range-checking, etc), which can happen as it grabs GameObjects, for example. The setter (and the complications you note with having it) have disappeared. You might be able to dispense with the getIntelligence method altogether, depending on the situation. Allen Holub takes this idea to its logical conclusion, but but that approach doesn't seem to be very common.

Rich Apodaca
That's exactly what I wanted to avoid!getIntelligence()setIntelligence()applyIntelligenceBonus()resetIntelligenceBonus()getStrength()setStrength()applyStrengthBonus()resetStrengthBonus()getWisdom()setWisdom()giveWisdomBonus()resetWisdomBonus()getDexterity()setDexterity()applyDexterityBonus()zeroDexterityBonus()getHP()setHP()dealDamage()healDamage()...You end up having a lot of redundant code doing the same operation on different fields. Maybe they aren't even consistently named.That's the definition of error prone, IMHO. :)
badp
A: 

Consider using a "modeling" framework, such as Eclipse's EMF. This involves defining your objects using something like Eclipse EMF editor, or in plain XML, or in Rational Rose. It's not as difficult as it sounds. You define attributes, constraints etc. Then the framework generates the code for you. It adds @generated tags to parts it added such as getter and setter methods. You can customize the code, and later edit it either by hand or via some GUI, and regenerate the java files.