Search code examples
javaencapsulation

Is it good to call instance attributes using getter and setter?

I'm making a battleship java game and throughout the entire process I've used the Point class to store X and Y coordinates. I know how getters and setters work and why we used them, but this specific class has both getters and public attributes so here's my question, what would it be better to use?

p.x or p.getX()

I know it's a good practice to use getters but I'm confused in this case, what should I use?

Solution

  • (to recap, this is about Immutable Objects vs changing primitive position values directly)

    I have to counter @rzwitserloot's answer. He has provided three problematic classes:

    1. The first one, the Lombok one, is either also immutable, or using @NonFinal or @PackagePrivate annotations, thus using (creating/allowing) unchecked getters and setters. So when it comes to how safe the code is, that either has the problems of immutability, or it's simply the same level as (directly accessible) public member variables.
    2. The second one, the record, is shallowly immutable. That means that any change to the position cannot be done to the Point itself, but each time a new instance of Point has to be created
    3. The third one, very similar to the second one, is a truly immutable class. Same here: any change to the position cannot be done to the Point itself, but each time a new instance of Point has to be created

    So what we see here: immutability. If you want to keep your code nice and clean, immutability is nice. Especially for debugging. Immutability is a fine tool for writing clean software. The dream of tinkerers. But for hands-on guys, in some situations, this soon becomes 'overengineering'.

    But with regards to performance there are some problems:

    1. For every change to an immutable class, you have to create a copy of that class, with a few changes
    2. It's harder to change only one aspect/value, then change another value later, without creating multiple objects

    So, assuming a stupid JVM,

    1. you'd soon run into memory-based performance problems, due to all those objects you need to create
    2. the speed is a horrible lot slower than simply changing the values.

    Gladly, the JVM is very smart:

    1. After a short runtime the JIT finds/knows classes that are used and discarded and optimizes that.
    2. Also, access via getters/setters is a lot slower, but the JVM also can take most of that away in most situations.

    Also, with the JVM advancing every year, chances are that the optimization/speed will increase further, reducing or maybe even annihilating any disadvantages of the immutable classes. Also, the optimizations will definitely be safer or at least as safe as anything you could design. But can they be as efficient?

    BUT there are also cases where the JVM can do neither optimizations. Especially when interfaces/abstract base classes are involved, even method calling can get slower, because addresses of the real targeted methods has to be resolved at runtime, each and every time.

    So in the end, it's up to you to test and decide, what approach you want to use. If this bit of added safety is really worth the 'drop' in performance. And how much you expect from the future, regarding JVM optimization.

    Where the others are right: don't use AWT classes unless you're really using AWT. Better have your own class that you can fit to your needs.

    // BIG UPDATE:

    One last thing to consider, and IMO the biggest downside to using an immutable Position type:

    (WARNING: This example will get more and more ridiculous! But it shows where that strategy leads)

    • Let's say we have the following Class: class Ship { Position position; }
    • So we address that position via ship.position
    • Now, when we want to change the ship's position, we have to change the reference to the position: ship.position = ... (new Position or ship.position.clone(newX,newY) or ship.position.with(newX,newY)
    • So whenever we want to change the ship's position, and follow the immutability pattern
      • semi-consequently: we would at least need another getter/setter for Position in Ship
        • Anything working with positions would also have to know its containing Classes, for example Ship and any and all other things that have a Position and might be computed by the same logic (yes, interfaces, but where does that stop?)
        • Also you'd have to check every time if the Ship.position is not null...
      • fully consequently: the Ship should also be immutable, and on any change to its position or any other status, the Ship in the Scenario would be replaced by a muted immutable copy.
        • Also, the Scenario would have to be replaced on any change. Wow.
          • Anything working with the ship would also have to know all the References TO the Ship, so it can update those
          • If the Scenario changes, the Game should also be immutable and its instance has to be replaced.. wait, what?
        • So we see that there exist a lot of scenarios where immutable Objects are not a good solution