8

We have entities with numeric properties, they are of boxed types (e.g. Integer). Typically, each of those properties has a pair of getters: getInt(), which returns zero on null values, and getIntNullable(), which returns values as is.

// we have a bazillion entities like this one
public class SomeEntity {
 public Integer someInt;
 /**
 * @deprecated Maps nulls to zero
 */
 public int getSomeInt() {
 return someInt == null ? 0 : someInt;
 }
 public Integer getSomeIntNullable() {
 return someInt;
 }
}

getInt() type of methods are marked @Deprecated. I guess it's because there were some bugs related to that in the past. It's a mature project. But getInt() methods come in handy in some cases: for example, when we need to pass zero for newly persisted entities. It makes no apparent sense to call getIntNullable() and then map it to zero if we can get zero directly from the instance right away (for example, using Optional.ofNullable(nullableInt).orElse(0)). Besides, replacing getInt() with getIntNullable() may cause (and did cause) NPEs on unboxing. Those kinds of issues can sometimes be tricky to spot and may not immediately emerge. But any time we use those deprecated methods, we get shamed by the IDE.

The only reason I see for those methods to be deprecated is getters, I believe, are supposed to return values as is. Including any mapping logic in them feels kind of wrong. That argument may be a bit too theoretical, though.

One solution is to rename getIntNullable() to getInt() and to rename getInt() to getIntOrZero(), removing deprecation. It's going to be a ton of work (on the other hand, it's going to be a ton of work either way, if we are to get it right). Also, may be a bit confusing to those used to old naming patterns.

What's your advice? Can getters perform that sort of mapping — or should it be a taboo in software engineering?

asked Mar 13 at 11:29
13
  • 29
    The whole point of getters is that they (can) contain mapping logic, otherwise you'd just access the public .someInt directly Commented Mar 13 at 22:49
  • 7
    Yes, so why would you think there's a taboo on including some logic? What's the difference between "now" and "later on"? Commented Mar 14 at 10:14
  • 7
    A getter is a part of the public interface of a class. The public interface defines how the class communicates with the outside world, allowing you to change the underlying (internal) representation if you have a reason to. A getter is no different in that regard - the value you return is the value that's meant to be visible to the outside, it doesn't need to be exactly the same as the underlying field (or a combination of fields); it can also be computed without there being a corresponding field. Commented Mar 14 at 15:22
  • 4
    Why the heck is "someInt" declared as public? Getters to encapsulate fields make only sense when the fields are private. Or was this a type of yours? Please clarify! Commented Mar 15 at 6:35
  • 1
    Method references, debugging what? Commented Mar 15 at 10:08

7 Answers 7

25

Must getters return values as is?

No. You are allowed to practice data hiding and encapsulate beyond simply running access to fields through behaviorless getters and setters.

We have entities with numeric properties, they are of boxed types (e.g. Integer). Typically, each of those properties has a pair of getters: getInt(), which returns zero on null values, and getIntNullable(), which returns values as is.

Java is one of the languages victimized with mandatory nullability for every object. Do not confuse this language "feature" with one of your application features. Often this "feature" isn't helping you.

Your implementation of your getInt() does a fairly reasonable narrowing conversion from Integer to int. The fact that you chose to back getInt() with an Integer is none of the outside world's business.

However, public Integer getSomeIntNullable() is a semantic crime. Yes Integers are nullable but Nullable is a type of its own (javax.annotation.Nullable) and you aren't returning it, you're returning an Integer. A better name would be getSomeAsInteger(). Or simply getSome(). Hungarian type warts went out of style a while ago.

Some others seem to think you really should return an Optional. I'm just going to point out how that isn't an absolute fix since Optional is an object that also can be null. Optional is a semantic trick that says this thing might not exist and that's ok. Use it when not existing is ok. The fact that in Java every object might not exist is Java's problem. It's not something your semantics have to support.

int is a primitive. It can't be null. It can be uninitialized. It models that state by being zero. It models being zero the same way. So there ain't any perfect way to model here.

However, if the expected behavior is to get a zero when absent/uninitialized then using int simplifies a lot since it does that already.

If the possible meaningful values are all non-negative, a classic int trick is to use -1 as a flag that says the int value isn’t meaningful. So Optional and Integer aren’t the only ways to model absence. They just offer more meaningful values since null doesn’t have to be encoded in the int.

But what you do inside your class is your own damn business. Just give me what I need. Make me expect what I get. I don't care about how. As long as it works.

answered Mar 13 at 14:29
0
14

This is a bit of an X/Y problem and my answer has just a pinch of Frame Challenge.

I see some hints at the solution and the real problem:

Besides, replacing getInt() with getIntNullable() may cause (and did cause) NPEs on unboxing.

...

It makes no apparent sense to call getIntNullable() and then map it to zero if we can get zero directly from the instance right away (for example, using Optional.ofNullable(nullableInt).orElse(0)).

...

One solution is to rename getIntNullable() to getInt() and to rename getInt() to getIntOrZero(), removing deprecation.

I think the solution is to do all of the above. Caleth's answer is idiomatic for contemporary Java; if you have a "nullable" reference type, use an Optional. You clearly have use cases where an intelligent default exists (zero) and other use cases where the absence of a value is semantically meaningful.

  1. Change someInt to an OptionalInt type.
  2. Return someInt as an OptionalInt in getInt() and drop the deprecation.
  3. Add a new method called getSomeIntOrDefault() which returns zero when someInt is not present.
  4. (maybe) add an overload of getSomeIntOrDefault() that allows you to provide your own default.

You end up with:

OptionalInt a = bar.getSomeInt(); // no more NullPointerExceptions
int b = bar.getSomeIntOrDefault(); // returns zero if not present
int c = bar.getSomeIntOrDefault(10); // returns 10 if not present

Now your public API clearly indicates the intended use cases.

I do have to agree that the existing API is sub-optimal and a bit awkward, however I don't think the issue is "mapping things in a getter." The real issue is that you have use cases where the absence of a value is meaningful and other use cases that have an intelligent default when the value is not present. There is nothing wrong with an object providing getters to satisfy both use cases. Just make sure the name of the getter tells the humans why you would call it. Failing that, add a good JavaDoc comment.

Since this sounds like a large code change, consider splitting this into two bodies of work: 1) add the getSomeIntOrDefault() method, and call it everywhere you need a default; 2) Change someInt from Integer to OptionalInt and update all the places it gets called (while also removing the @Deprecated attribute).

And yes, this will be grunt work, but they are only compiler errors. They can be fixed. This at least gets rid of the runtime NullPointerExceptions.

answered Mar 13 at 12:59
9
  • 2
    Why getSomeIntOrDefault() over getSomeInt().orElse(0)? Commented Mar 13 at 13:39
  • 5
    If the object owning the property has a default which is not 0. And which the caller is not supposed to know. Commented Mar 13 at 13:52
  • 2
    @JacobisonCodidact: OrElse lives on the return type of getSomeInt, which means that getSomeInt is not returning a straight up integer anymore. This means that any consumer who isn't using a follow-up method after calling getSomeInt has to now manually deal with and unwrap the object that's being returned in order to find the integer value within; which is a bad experience for consumers of your code. Sure, it might be as simple as doing .Value, but if this is how you design your getter methods, you will start peppering your entire codebase with .Value everywhere. Commented Mar 14 at 0:12
  • 3
    @Flater the answer already suggests returning an OptionalInt so I don't think your objection applies Commented Mar 14 at 7:01
  • 1
    @JacobisonCodidact: The point I'm making here is that the unwrapping is either oart of the same method or part of a chained method. In the chained method case, that softly forces consumers to either always call a chained method or do the unwrapping themselves, which can become a lot of work when you consider how frequently your application will be using getters in its codebase. Commented Mar 14 at 21:44
12

The only reason I see for those methods to be deprecated is getters, I believe, are supposed to return values as is. Including any mapping logic in them feels kind of wrong. That argument may be a bit too theoretical, though.

Getter/setter pairs that do nothing but transparently pass a value around are essentially pointless. You might as well just expose the field value. You can argue that it leaves you options to do something in the future without disrupting all the code that uses it. But if you forbid any such logic from ever being used, then that argument makes no sense.

In that regime, there isn't much about getters/setter pairs that make them useful over bare fields. Off the top of my head:

  • You can have different access levels for the getter and setter. For example, you can make the setter protected while leaving the getter public
  • You can add 'passive' code and/or annotations to the setter (or, less commonly, the getter) such as observer notifications

While these can be valuable features in some cases, I think there is far too much code where the use of simple, transparent getters and setters are a rote habit based on old (bad, IMO) ideas that almost no one understands or cares about anymore.

What's your advice? Can getters perform that sort of mapping — or should it be a taboo in software engineering?

If you make a field private and restrict access to it through getters and setters, those getters and setters are part of the public interface (in the general sense) for that class. The whole point of abstraction is that the users of the interface should not care or need to know how that interface is implemented. The idea that this interface must be implemented in a specific way is entirely inconsistent with that principle.

However, this 'pattern' is often associated with procedural programming masquerading as object-oriented programming. If your team is operating that way, introducing something other than a direct transparent pass-through in a getter or setter might violate the 'principle of least surprise' for developers who are sure that transparent getters and setters are the right way to do things because that's the only way they've ever seen it done. Personally, I think the expectation is wrong, but that is a people problem, not a technical one.

As far as mapping null to 0 goes in these methods, it seems like a useful convenience to me, but your description how widely this is used suggests to me that the design suffers from primitive obsession.

answered Mar 13 at 15:48
10
  • 2
    C++ often combines a private instance member variable with a public getter/setter method pair. If you say it’s pointless I won’t contradict you, but that is a very very common pattern in C++. Commented Mar 14 at 14:50
  • I don't see the connection to "primitive obsession". Commented Mar 15 at 10:03
  • @gnasher729 It's also common future-proofing in Java because it doesn't have properties (assuming it hasn't changed since, I'm not up-to-date on the language) Commented Mar 16 at 6:41
  • @user3840170 There's a comment at the top of the code "we have a bazillion entities like this one" which suggests to me that there's a lack of proper modelling. A plethora of property bags populated with and presenting primitives. Commented Mar 17 at 15:47
  • Maybe there is bad modelling in the project, but I see no evidence of this particular kind. No need to name-drop anti-patterns as pure speculation. Commented Mar 17 at 18:36
7

If someInt can be absent, then use OptionalInt. If it can't, then use int.

Having a field of type Integer is just weird. The boxed types exist to allow primitives in places where Object is expected.

However the name someInt is awful, especially if it can be absent. I hope your actual properties have sensible names, for which a determination between getFoo() and getFooOrZero() is simple for the user of this class.

answered Mar 13 at 12:43
2

Lots of valuable answers already, but I'd like to stress the importance of modelling unknown information:

For the "someInt" information that gets represented in that field, there seems to be a notion of "I don't know the value". E.g. if "someInt" is "age", then "unknown" should always be distinguishable from "0 years", and none of the APIs should hide that distinction.

Your SomeEntity class having an Integer field instead of an int hints at that, and the "@deprecated Maps nulls to zero" also supports that interpretation.

But getInt() methods come in handy in some cases: for example, when we need to pass zero for newly persisted entities.

There's the problem (if my interpretation is correct). The "need to pass zero" breaks the model, as it would falsely declare e.g. the newly-persisted person to be a new-born baby instead of an unknown-age person.

Information Technology is all about handling information, and not having some information is quite normal, it happens all the time. So our modelling should almost always include some representation of "unknown" and be careful not to confuse it with some other, possibly valid value.

It's of minor importance how you represent "unknown". Older Java code typically uses null, later the Optional class became popular. I'd only stay away from "out-of-range" special values, e.g. -1 for unknown.

Besides, replacing getInt() with getIntNullable() may cause (and did cause) NPEs on unboxing. Those kinds of issues can sometimes be tricky to spot and may not immediately emerge.

It's important to spot these bug places where a consumer of your class falsely assumes that someInt has a known value, ignoring that it can be unknown. And that's the advantage of Optional: it's easier to spot the places where a consumer tries to access the value.

answered Mar 14 at 8:29
1
  • 1
    Interesting when you want to model that the age is unknown, or secret, or needs reading from a database, and whatever else you can think of. Commented Mar 14 at 18:50
2

Getters are not obliged to return a value as-is. In fact, their entire existence stems from the desire to do something other than returning a value as-is. So it's totally fine for a getter to do something more complicated.

For example, I might have a class which lazily calculates something. Maybe this thing I'm calculating depends on another value that changes all the time, but I only ask for this calculated value once in a blue moon. Instead of redoing the calculation every time the underlying value changes, and shoving the calculated result into a variable to be returned as-is, the class might keep track of whether its inputs are "dirty," meaning they changed since the last time the calculation was undertaken. The getter might check if the value is "dirty" and recalculate it if it is - and then return the value.

The purpose of getters is to hide such details. If you don't need to know whether a value is being calculated in such way, you can use a getter. If you know for certain that the value is returned as-is, simply having a public member might be easier.

That being said, the example points to another problem. getInt() vs getIntNullable raises code smell issues. assuming the value is nullable (or optional, as other answers have suggested), having getInt() return 0 if the value isn't present is a decision that you need to think long and hard about. Make sure that's the right answer. Returning 0 if the value is not present is identical to saying "as far as callers of getInt are concerned, 0 and null are identical." Often this is not the right answer.

I say "often" because that's sometimes how it works. I have seen APIs that say things like "if an error occurs, this function returns 0. To distinguish between this function processing 0 bytes of data and an error occuring, one should call GetLastError." This is popular in systems which do not use exception handling for one reason or another.

A more common variant of what you wrote is to have getInt() throw an exception if the value does not exist. The logic here is that in many situations, the caller of getInt is assuming that the int exists, and its non-existence is a very exceptional circumstance. For algorithms where assuming the int is present is a poor assumption, those algorithms can use getIntNullable().

answered Mar 20 at 5:49
5
  • 1
    It might be a naming issue. If I call getSomeInt(), I expect to get someInt, not some primitive substitute Commented Mar 20 at 6:45
  • 2
    If the purpose of getter is not to get, then why call them such? Just apply "tell don't ask" principle and name accordingly - with verbs, not with prefixed nouns. Commented Mar 20 at 6:55
  • @Basilevs I'd argue "get" is a verb, so it does use that naming scheme accordingly. As to the more philosophical aspect of your question, I'd say getters exist because sometimes "getting the data" is precisely the abstract operation you are looking for, but without worrying the caller about the details. I don't think Java really pushes that paradigm as much as other languages. In C# and Python, for example, getters are so much "get the data" that they permit using the exact same syntax as getting a data field (obj.someInt). Java just doesn't go that far. Commented Mar 20 at 7:43
  • Ah, would you then argue that "Doer" is a good noun for a class name? JK Commented Mar 20 at 11:27
  • I agree, that sometimes data is just that. But it is a bad idea to split it from code. So leave true getters in DTOs, or just use public properties there. Do not mix DTO and OOP. Commented Mar 20 at 11:28
0

Here's my heretical answer: Getters should be rare and setters almost non-existent. You're worried about the wrong problem. You need to refactor your code so the class is responsible for its internal state and methods that modify that state do something related to the class other than exposing its internals.

Yes, I know some Java tools require getters and setters to do what they do. That's a code smell.

answered Mar 19 at 16:14
1
  • 1
    Yep, getters should not even be considered. Commented Mar 19 at 18:20

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.