4

I'm a junior developer (currently exercise Java) and I have one question about the correctness of my code, here is an example: I am writing a simple MMO-game representation on Java, I have 2 classes (Character and spell). Character has properties (mName, mHealth and mEnergy), Spell class has properties (mSpellName, mSpellCost, mSpellDamage). And Spell class also have a method called execute, here is a code

public void spellExecute(Character caster, Character target) {
caster.mEnergy -= this.spellCost;
target.mHealth -= this.spellDamage
}

This construction implies that Character fields are public and can be accessed directly, but in some examples I seen that all fields must be private and can be accessed only via get/set methods. My question is: Which way is more correct, in general? It's important to me because I wanna write a good code :)

  • 1
    Getters and setters are more frequently thought to be correct in my experience, even if you don't do any intermediary processing on the get/set – Kon Feb 16 '15 at 16:42
  • Getters and setters have the advantage that you don't have to change your design if you have to update your class after setting the value. – MinecraftShamrock Feb 16 '15 at 16:43
  • 3
    public attributes are evil. Every quality check tools will spot it. – Arnaud Denoyelle Feb 16 '15 at 16:43
  • 1
    possible duplicate of [Are getters and setters poor design? Contradictory advice seen](http://stackoverflow.com/questions/565095/are-getters-and-setters-poor-design-contradictory-advice-seen) – MinecraftShamrock Feb 16 '15 at 16:44
  • 1
    possible duplicate of [Why use getters and setters?](https://stackoverflow.com/questions/1568091/why-use-getters-and-setters) – MinecraftShamrock Feb 16 '15 at 16:45
  • Please refer to [this wonderful explanation][1] [1]: http://stackoverflow.com/questions/1568091/why-use-getters-and-setters – antonio Feb 16 '15 at 16:45
  • Off-topic: you shouldn't name your class with same name which is already used. Java has its own `java.lang.Character` class which is used as wrapper of `char` type. This will lead to many problems. – Pshemo Feb 16 '15 at 16:54
  • Have you considered using someone elses Role Playing engine? There seem to be a few around. – Richard Feb 16 '15 at 16:57
  • @Richard I think he's just exploring the realm of OOP by playing around with some common scenarios that can be represented by classes and objects. – hhanesand Feb 16 '15 at 17:00
  • @lightice11 Yes I understand. It's a great idea to learn OO from too. I just thought if he were serious about writing a game, he might like to consider using someone elses engine. All these role playing games are very similar. You buff skill or other using experience, or enchanted weapons etc. – Richard Feb 16 '15 at 17:02

7 Answers7

2

You're right that it's important to write good code and at first getting the grasp of Object Oriented Programming can be a bit difficult.

In this case, I would recommend moving the spellExecute to a similar method, except on the Character class :

public void didUseSpell(Spell spell) {
    this.mEnergy -= spell.spellCost;
}

public void wasHitBySpell(Spell spell) {
    this.mHealth -= spell.spellDamage;
}

In your spell execute method, you would then call :

public void spellExecute(Character caster, Character target) {
    caster.didUseSpell(this);
    target.wasHitBySpell(this);
}

In general, there are many different was of solving this problem, and they all vary in terms of code cleanliness and verbosity. Another solution would be to create getter and setter methods for the fields that are affected by the spells.

Community
  • 1
  • 1
hhanesand
  • 990
  • 11
  • 28
2

Generally, you would use get/set methods, because they allow the class to control access, via those methods

Any other class which uses your class, should only be able to access and change your fields in the way you describe.

For example let's look at a simple (oversimplified) fuel pump

class Pump
{
    constant int PRICE_PER_LITRE = 100; //in pence

    private int litresDispensed;
    private bool nozzleUp;
    private int litresAvailable;
    private int price; //in pence

    construct()
    {
        litresDispensed = 0;
        nozzleUp = false;
    }

    startFuelling()
    {
        nozzleUp = true;
    }

    stopFuelling()
    {
        nozzleUp = false;
    }

    takeFuel(int litresTaken)
    {
        if(nozzleUp = true)
        {
            litresAvailable -= litresTaken;
            price += litresTaken * PRICE_PER_LITRE;
        }
        else
        {
            error("You must lift the nozzle before taking fuel!");
        }
    }

    getPrice()
    {
        if(nozzleUp = true)
        {
            error("You can't pay until you've finished fuelling! Please return the nozzle");
        }
        else
        {
            return price;
        }
    }
}

Our final get method is important to ensure that the rest of the transaction is complete before the person tries to pay.

If we allowed direct access to the price, they could do it before they've finished taking fuel! And that would let them steal all our fuel.

As this shows, a get method protects the field from outside influence. It can still be manipulated, but only in the ways we want to allow it to be manipulated. Note also that there's no set method at all for this field: we don't want the user to be able to set their own price, only the one we dictate!

If you write get/set methods which only return and set the field, without any validation or checks, then you could simply make the field public (or, alternately, you need to decide whether that field should be accessed directly at all): that said, it's good practice to use get/set methods where possible, because it allows you to add validation in the future without breaking code.

Jon Story
  • 2,881
  • 2
  • 25
  • 41
1

Getters/setters are better, because it encapsulates or hides what the actual class is doing to set that data. Set the constructor private and let it initialize default values, then the user can call the set methods to set the variables.

Lawrence Aiello
  • 4,560
  • 5
  • 21
  • 35
1

Getter and setter (Java bean) is more better.It also provide Encapsulation feature.Which is useful for hiding data.

  private int id;

    public int getId() {
            return id;
    }

    public void setId(int id) {
           this.id = id;
    }

Constructor is used for initialization the value when You are creating object.But If you want to set value of data after object creation then you must call setter behavior instead of call constructor.

Arun Kumar
  • 2,874
  • 1
  • 18
  • 15
1

Getters and setters with private fields is generally followed design choice. The reason is, you can guard your variables against unintentional changes from the clients using your API. Consider this below example

public class Dummy{
 public int age;
}

Now client can do this

new Dummy().age = -1

With setters and getters

public class Dummy{
   private int age;
   public void setAge(int age){
      if (age < 0){
       throw new RuntimeException("you are not allowed to do this");
   }
   ......
}

Many frameworks for example Hibernate, IBATIS etc.. follow these naming conventions. Hope this answers your questions.

JavaMan
  • 351
  • 1
  • 3
  • 13
0

Getters and setters are preferred - after all, why use an Object Orientated language if you're not going to use its main feature?

With setters in this situation you can easily enforce sanity rules without each caller having to duplicate the logic, e.g. - in Character try:

public void reduceMentalHealth(int val) {
  if(this.mHealth > val) {
    this.mHealth -= val;
  } else {
    this.mHealth = 0;
}

Without setters you would need this logic everywhere you changed the field. You could also include things like checking whether the Character is wearing a ring of mental invincibility in this method too :-)

BarrySW19
  • 3,759
  • 12
  • 26
0

Warning

You are mixing two related questions.


Questions:

[1] Should I use direct access fields, or fields access by accesors methods ( "getter" (s) and "setter" (s) ).

[2] In both cases, which access modifiers, should apply ?


Short Quick Answer

[1] Should I use direct access fields, or fields access by accesors methods ( "getter" (s) and "setter" (s) ).

Go, for the "fields access by accessor methods" a.k.a. "getter (s) and setter (s)". The other method is NOT wrong, it depends, what do you want to do.

[2] In both cases, which access modifiers, should apply ?

If you go for "plain fields", use "public", unless there is an specific reason to use "private" or "protected". If you go for "accesors methods", use "protected" for the fields, and "protected" or "public" for the accesors.


Long Boring Extended Answer

[1] Should I use direct access fields, or fields access by accesors methods ( "getter" (s) and "setter" (s) ).

Go, for the "fields access by accessor methods" a.k.a. "getter (s) and setter (s)".

The other method is NOT wrong, it depends, what do you want to do.

But, since property access can be overriden by those methods, that allows more features to be added, removed or changed by methods.

I suggest, leave "plain fields" for Data Objects.

[2] In both cases, which access modifiers, should apply ?

If you go for "plain fields", use "public", unless there is an specific reason to use "private" or "protected".

If you go for "accesors methods", use "protected" for the fields, and "protected" or "public" for the accesors.

Is not a good idea to apply "public" access for accesors' fields, because this way, you confuse yourself and other programmer users of your classes, on which one to directly use.

Code Example:

public class ControlClass {
    // example of non private field (s)
    protected String controlname;

    // ...

    // example of non private field (s)
    protected int width;
    protected int height;

    // ...

    // example of read-only property
    public final String getControlName()
    {
        return this.controlname;
    } // String getControlName(...)

    // ...

    @Override
    public int getWidth()
    {
        return this.width;
    } // int getWidth(...)

    @Override 
    public void setWidth(int newvalue)
    {
        this.width = newvalue;
    } // void setWidth(...)

    @Override
    public int getHeight()
    {
        return this.height;
    } // int getHeight(...)

    @Override 
    public void setHeight(int newvalue)
    {
        this.height = newvalue;
    } // void setHeight(...)

    // ...

    // constructor assigns initial values to properties' fields
    public ControlClass(){
        this.controlname = "control";
        this.height = 0;
        this.width = 0;
    } // ControlClass(...)

    // ...
} // class ControlClass

public class ButtonClass extends ControlClass {
    // ...

    // example of overriden public property with non public field
    @Override
    public int getWidth()
    {
        if (this.width < 5)
          return 5
        else
          return this.width;
    } // int getWidth(...)

    // example of overriden public property with non public field
    @Override 
    public void setWidth(int newvalue)
    {
        if (newvalue < 5)
          throw new ArithmeticException("integer underflow");

        this.width = newvalue;
    } // void setWidth(...)

    // example of overriden public property with non public field
    @Override
    public int getHeight()
    {
        if (this.height < 5)
          return 5
        else
          return this.height;
    } // int getHeight(...)

    // example of overriden public property with non public field
    @Override 
    public void setHeight(int newvalue)
    {
        if (newvalue < 5)
          throw new ArithmeticException("integer underflow");

        this.height = newvalue;
    } // void setHeight(...)

    // ...

    // constructor assigns initial values to properties' fields
    public ControlClass(){
        this.controlname = "ButtonClass";
        this.height = 5;
        this.width = 5;
    } // ButtonClass(...) 

    // ...
} // class ControlClass

I started to use "private" for fields, like many programmers, buy, eventually, had to change to "protected", because sometimes required to use it. directly.


Additional Comment.

I have work with other object oriented programming languages, with its own way and syntax for "properties", and that's give you another perspective.

Such as Object Pascal ( a.k.a. "Delphi" ), ECMAScript ( "Javascript" ), C++, C#. Note that Delphi and C#, supports "full properties", not just accesors methods or fields, and that's give developer another way of designing an Object and Class Oriented Software Application.

What does this has to do with Java ?

We'll when I design a Class in Java or C++, I design properties, the same way, as C# or Delphi, does, a concept that is independent of fields, or methods, even if there can be implemented by them.


Cheers.

umlcat
  • 4,091
  • 3
  • 19
  • 29