CODEWITHSUNDEEP
javagetter
Stezy
Stezy

Reputation: 390

Is it good practice to define a getter which does not return a specific field?

a kind of simple question. Are methods like this good practice?

private NavigableMap<Double, Site> m_sites;

private Site getRandomSite()
{
    return m_sites.ceilingEntry(Math.random()).getValue();
}

Or is it bad because it is a getter and you could expect that it would return the field "randomSite"?

Upvotes: 4

Views: 711

Answers (6)

Markus Kull
Markus Kull

Reputation: 1477

There are some conventions regarding getters. If there is a getter, there may be a corresponding setter. This setter must take one parameter with exactly the same type as the getter. Some Frameworks e.g. Spring may enforce this.

Back to your Question, it is expected that getters return a field at last. This doesnt mean that the field must be directly returned. There may be e.g. lazy-loading or conversion (getDurationInMillis, getDurationInSeconds,...), but is it really expected that get-Methods return the same value if no change is done. Math.random() seems to be a prime example for this. It is called random() instead of getRandom() because changing values are returned.

In conclusion, if a method doesnt fulfill all getter-expectations, it is probably better to name it otherwise. fetch, search, ...

Upvotes: 0

NoDataFound
NoDataFound

Reputation: 11969

If your method does compute something, then probably a method name that explicits this behavior is better:

private NavigableMap<Double, Site> m_sites;

private Site pickRandomSite()
{
    return m_sites.ceilingEntry(Math.random()).getValue();
}

In any case, whether it's a real getter (that is a dummy method for the lacks of properties like in C# or the fear of public fields for struct equivalent) or not, the only thing that matters is that the method name reflects its purpose.

Example: don't call getSite() a method that would return a random site.

On a side note, what bugs me - more than the use of getRandomSite - is the m_sites. I know after asking that this is a company convention, but it's like obfuscation to me (I hate the prefix member/method by its scope naming convention).

Upvotes: 0

CodeNewbie
CodeNewbie

Reputation: 2091

The method is fine, although I can understand that you wouldn't want to call it getRandomSite() because it looks like a getter method. Building on Gio's answer, I suggest you call the method fetchRandomSite() because as you said, this method does not generate the random site, it simply picks (or fetches) it out of a NavigableMap.

Upvotes: 4

kiwiron
kiwiron

Reputation: 1705

No, it is bad practice. Methods like that should be static, and encapsulated in utility classes, as they have nothing to suggest that they are associated with the enclosing class. In addition, the utility class should have a private constructor so it cannot be accidentally instantiated.

A situation where they may be useful is as MyClass.allocateNextUniqueKey(). But in this case the method should not allocate an already used key, so it most definitely has an association with MyClass.

Upvotes: 1

Gio
Gio

Reputation: 3340

If you, the author of the code, think it is confusing / bad with respect to the rest of your code base, than change the name to for example generateRandomNumber(). However, there is no convention which says that a "get method" must always return a field.

Upvotes: 4

Suresh Atta
Suresh Atta

Reputation: 122018

A getter can get anything in back (return anything). Doesn't mean that it return only some field's value. No issues with your current method.

Upvotes: 3

Related Questions