experchange > java

Rhino (10-24-18, 11:04 PM)
Is it better to overload a single method name several times or to create
uniquely-named methods to accomplish similar work?

I am in the midst of rewriting some old convenience methods that work
with dates and times but use the old Calendar and java.util.Time
classes. I know that it is far better to use the java.time.* packages now.

In the course of revising my methods to use the Java 8 classes, it has
occurred to me that I should have several variants of some of the
methods. For instance, one of my methods determines the name of the
current month.

It seems reasonable to me that I might at some point need to know the
name of the current month:
- in either short or long version - i.e. "Oct" or "October"
- in the local timezone or a different one (after all, while it may be
late in the day on Oct 31 in my timezone, it may already be early Nov 1
a few timezones away)

APPROACH 1:
I could have several overloaded versions of a single method. For instance:
- getCurrentMonthName(boolean fullMonthName) and assume local time zone
- getCurrentMonthName(ZoneId zoneId) and assume full month name
- getCurrentMonthName(boolean fullMonthName, ZoneId zoneId) and assume
nothing
- getCurrentMonthName() and assume local time zone and full month name

APPROACH 2:
Or I could do this:
- getCurrentMonthNameLocal(boolean fullMonthName) and assume local time zone
- getCurrentMonthNameRemote(boolean fullMonthName, ZoneId zoneId)
and assume nothing

APPROACH 3:
I could even do this:
- getCurrentMonthNameFullLocal() and assume local time zone
- getCurrentMonthNameShortLocal() and assume local time zone
- getCurrentMonthNameFullRemote(ZoneId zoneId) and use specified time zone
- getCurrentMonthNameShortRemote(ZoneId zoneId) and use specified time zone

APPROACH 4:
Or I could reduce the number of methods I need by having only this:
- getCurrentMonthName(boolean fullMonthName, ZoneId) which would force
users to supply a value for the boolean and the zoneId every time even
if they wanted the local time zone

Which is the best design and, most importantly, WHY? I expect there is a
basic design precept somewhere that helps a developer decide which is
the best way to go in circumstances like these. It would be helpful to
learn (or rediscover) this principle.

Can anyone help me out?

In the absence of such a principle, it seems to me that I need to weigh
the costs of each approach against the benefits and see which is the
best balance. For instance, Approach 4 has the benefit of requiring me
to code only one method to get the current month but means the method is
more complex and probably has more conditions to test in unit test. It
may also be more work for the person trying to use these classes as they
have to understand the meaning of the parameters and figure out how to
set the desired values. (I'm just learning the Java 8 time classes and
didn't know what a zoneId was 24 hours ago, let alone how to set it and
what values I could use. A developer using these classes might be in the
same boat.)
Arne Vajhj (10-25-18, 12:56 AM)
On 10/24/2018 5:04 PM, Rhino wrote:
> Is it better to overload a single method name several times or to create
> uniquely-named methods to accomplish similar work?


[..]
> basic design precept somewhere that helps a developer decide which is
> the best way to go in circumstances like these. It would be helpful to
> learn (or rediscover) this principle.


I would go for approach #5:

getCurrentMonth(boolean fullMonthName)
getCurrentMonth(boolean fullMonthName, ZoneId zoneId)

My logic:
* I expect the method name in a get to explain what it is getting
and any arguments to qualify how it should be getting it and
that points to #2 and
* and ZoneId is sufficient rare to warrant a default value and
since Java does not have defaults that means an overload

Arne
Joerg Meier (10-25-18, 10:13 AM)
On Wed, 24 Oct 2018 17:04:27 -0400, Rhino wrote:

> Is it better to overload a single method name several times or to create
> uniquely-named methods to accomplish similar work?


I would argue that you should overload methods that return the same thing,
but have separate methods for returning different things. In that spirit, I
would overload the methods with an without a zone, but have separate
methods for gettting the full name, as I suspect from your posting that
those do in fact return something different.

That being said, while this was probably just an example, I should point
out that this is generally the completely wrong approach and kind of
violates sensible object oriented design: return the month instead, and let
the caller ask the month what it's called. That's what java.time.Month is
for.

Liebe Gruesse,
Joerg
Eric Sosman (10-25-18, 01:49 PM)
On 10/24/2018 5:04 PM, Rhino wrote:
> Is it better to overload a single method name several times or to create
> uniquely-named methods to accomplish similar work?
> [...]
> APPROACH 1:
> I could have several overloaded versions of a single method. For instance:
> - getCurrentMonthName(boolean fullMonthName) and assume local time zone
> [...]


In addition to what Arne and Joerg have said, I'd suggest that
the association `true' -> "full name" and `false' -> "short name"
is not very self-documenting. Perhaps you should use an `enum'
instead, either TextStyle or something of your own, since

String name = getCurrentMonthName(TextStyle.SHORT);

is more easily read and understood than

String name = getCurrentMonthName(false);

Also, if you someday need to support extraterrestrial locales
and find out that Klingon has *three* common forms for month names,
an `enum' can always grow a third instance while a boolean can't ...
Rhino (10-25-18, 09:53 PM)
On 2018-10-25 4:13 AM, Joerg Meier wrote:
[..]
> violates sensible object oriented design: return the month instead, and let
> the caller ask the month what it's called. That's what java.time.Month is
> for.

Actually, it was a "real life example". That is one of the methods I am
rewriting
in my class of convenience methods.

I was intrigued by your suggestion that I should just return the month
and let the caller ask the month what it is called. I wasn't sure what
you mean by that. but I wrote a bit of code and came up with this:

static java.time.Month getCurrentMonth() {

LocalDateTime ldt = LocalDateTime.now();
ZonedDateTime zdt = ZonedDateTime.of(ldt, ZoneId.systemDefault());

return zdt.getMonth();
}

Then, in the code which calls the convenience method, something like this:

String fullMonthName =
CurrentDateTimeUtils.getCurrentMonth().getDisplayN ame(TextStyle.FULL,
new Locale("de", "DE"))); //gets full month name in German e.g. Oktober
String shortMonthName =
CurrentDateTimeUtils.getCurrentMonth().getDisplayN ame(TextStyle.SHORT,
new Locale("de", "DE"))); //gets short month name in German e.g. Okt.
int numericMonth = CurrentDateTimeUtils.getCurrentMonth().getValue();
//gets numeric value of month e.g. 10 for October

Or did you have something different in mind?

I rather like the approach you're suggesting. If I give the
responsibility of deciding which version of the month name people want
back to the calling program, that simplifies my convenience class so
that I need fewer methods while leaving it entirely possible for the
calling program to get the month information in any format it likes
since all the possible formats can be derived from what the convenience
method returns.

Either way, the program using my convenience method has to decide what
it wants - and will get what it wants - so it's very reasonable to
design the convenience class the way you suggest.
Rhino (10-25-18, 10:07 PM)
On 2018-10-25 7:49 AM, Eric Sosman wrote:
[..]
>     Also, if you someday need to support extraterrestrial locales
> and find out that Klingon has *three* common forms for month names,
> an `enum' can always grow a third instance while a boolean can't ...

Yes, I'd already thought about the lack of flexibility in the boolean
approach before I saw your and Joerg's replies. I created an Enum called
MonthNamePattern and gave it all 8 values possible for a Month,
acccording to the documentation in DateTimeFormatter: MMMM, MMM, MM, M,
LLLL, LLL, LL, and L.

I could just pass that Enum to the method to have it return the exact
format the calling program wants or, better yet (if I understood Joerg
correctly) just let the convenience method return the month value in a
java.time.Month and then let the *calling* program decide if it wants
the TextStyle.FULL or TextStyle.SHORT or the numeric equivalent of the
month. I could even use my Enum with a DateTimeFormatter to make the
calling program display the month using the single M formatting code if
that became necessary.

If I return the java.time.Month, then I really only need two variations
of getCurrentMonth:
- getCurrentMonth() which defaults to the system default zone ID
- getCurrentMonth(zoneId) which uses the specified zone ID

I *like* this approach! No unwieldy names like
getCurrentMonthFullRemote() and only two simple methods to write. In
fact, the convenience methods are so simple they are very close to
trivial and may not even be worth writing....

(Thanks to Joerg's remarks, I just now discovered the existence of the
separate classes for Month, Day, etc. and the various techniques they
offer to present the information however you want them. I had really
just started looking at java.time.* this week after a few years away
from Java and hadn't explored all the classes yet.)
Eric Douglas (10-25-18, 10:58 PM)
On Thursday, October 25, 2018 at 3:53:15 PM UTC-4, Rhino wrote:
[..]
> design the convenience class the way you suggest.
> --
> Rhino


Yes, that looks right. The key to overload is the return value. Use the same exact method name and add parameters if you're returning the same value.. Use boolean where there are exactly 2 possibilities (print duplex? yes or no). Use enums where there could be more than 2 possibilities or the parameter itself is simply not clear. You can have the method return the month in formatted string if one of your parameters accepts an enum to tell it which format. In this particular case returning the month object from the java time class makes sense if it already has enums to tell it how to format.
Eric Sosman (10-25-18, 11:49 PM)
On 10/25/2018 4:58 PM, Eric Douglas wrote:
> [...]
> Use boolean where there are exactly 2 possibilities (print duplex? yes or no). [...]


Use booleans where there exactly two possibilities *and* where
their associations with `true' and `false' are flat-out obvious.

/** An old-fashioned binary-gendered person. */
public class Person {
/** A poorly-designed constructor API. */
public Person(Name name, boolean gender) {
...

Things would be a little bit better if `gender' were renamed to
`isMale' or `isFemale', but even with the improvement it's not great:

Person flawedSage = new Person(new Name("Kao", "Li"), false);

.... is not especially transparent.

Also, be careful to distinguish between "there are exactly two
possibilities" and "I have thought of only two possibilities." It
is perhaps instructive that the example given would not work for the
printer that sits on my desk at this moment, which has *five* modes
for duplexing:

One-sided

Two-sided (automatic, flip on long axis)

Two-sided (automatic, flip on short axis)

Two-sided (manual, flip on long axis)

Two-sided (manual, flip on short axis)

(In the "automatic" modes the printer marks one side but doesn't
spit the page out, then sucks it back in and does the second side.
In the "manual" modes it prints and ejects all the back sides, then
you re-insert the stack in the paper supply and it prints the formerly-
blank fronts. "Automatic" is more convenient, "Manual" can be faster
for long documents.)

Duplex-or-not is, as suggested, a Yes/No question -- but if the
answer is Yes the answer is incomplete, lacking essential detail.
A good API offers (relatively) straightforward ways to supply all
the necessary information.
Martin Gregorie (10-26-18, 12:29 AM)
On Thu, 25 Oct 2018 17:49:28 -0400, Eric Sosman wrote:

[..]
> `isMale' or `isFemale', but even with the improvement it's not great:
> Person flawedSage = new Person(new Name("Kao", "Li"), false);
> ... is not especially transparent.

In those circumstances I tend to define constants in the consuming class.
In the OP's case I'd probably change the boolean to an int named
nameFormat and declare the constants:

public static final int LONG 1;
public static final int SHORT 2;

and my code in the class would also use the SHORT and LONG constants
because:

(a) this way I don't have to create an enum and the javadocs will
document the values automatically
(b) both the method call and the internal code are easy to read
(c) the constant values have no hidden uses so the actual values used
don't matter as long as they're different
(d) if additional values are needed in future they're easy to add

I realise that this approach probably betrays my C background, but is
anything else about it wrong enough that it should cause me to use enums
instead?
Arne Vajhj (10-26-18, 02:08 AM)
On 10/25/2018 6:29 PM, Martin Gregorie wrote:
[..]
> I realise that this approach probably betrays my C background, but is
> anything else about it wrong enough that it should cause me to use enums
> instead?


Enum achives the same 4 goals (except obviously the first half
of #a - not having to define an enum).

And enum will be better/easier documented for method arguments.

And enum is type safe while int constants are not.

foobar(7 * LONG + SHORT);

will be accepted by the compiler but have undefined semantics.

Arne
Martin Gregorie (10-26-18, 02:37 AM)
On Thu, 25 Oct 2018 20:08:54 -0400, Arne Vajhj wrote:

> Enum achives the same 4 goals (except obviously the first half of #a -
> not having to define an enum).

Sure, though being defined outside a class that is the only place its
used (apart from when its supplied as a method argument) as we've been
discussing does separate each constant definition and its documentation
from the class definition and documentation

> And enum will be better/easier documented for method arguments.

Apart from the separation mentioned above. I see this as a disadvantage
though you may not.

> And enum is type safe while int constants are not. AFAIK that is not relevant if the constant is defined as static final.


> will be accepted by the compiler but have undefined semantics.

Agreed, but being basically a mildly paranoid C programmer I tend to
validate all arguments, including those created with a #define (C) or as
static final int (Java) and you really shouldn't be defining a constant
unless its one of the above or part of an enum.

So far, it all looks a bit like swings and roundabouts to me except, of
course, if your enum contains logic rather than being simply a list of
constants.
Arne Vajhj (10-26-18, 03:42 AM)
On 10/25/2018 8:37 PM, Martin Gregorie wrote:
> On Thu, 25 Oct 2018 20:08:54 -0400, Arne Vajhj wrote:
> Sure, though being defined outside a class that is the only place its
> used (apart from when its supplied as a method argument) as we've been
> discussing does separate each constant definition and its documentation
> from the class definition and documentation


I am not sure that I can follow that argument.

If you make the enum nested instead of top level then it will be in the
same source file at the same place as the int constants would have been.

And in the java docs you just click on the type to get to the java docs
instead of scrolling around searching for info you need.

>> And enum will be better/easier documented for method arguments.

> Apart from the separation mentioned above. I see this as a disadvantage
> though you may not.


public void foobar(MyEnum val)

is simpler than:

/**
* ...
* Note: val takes the values SomeClass.Const1, ..., SomeClass.Constn
*/
public void foobar(int val)

>> And enum is type safe while int constants are not.

> AFAIK that is not relevant if the constant is defined as static final.


It is as the example shown.

>> will be accepted by the compiler but have undefined semantics.

> Agreed, but being basically a mildly paranoid C programmer I tend to
> validate all arguments, including those created with a #define (C) or as
> static final int (Java) and you really shouldn't be defining a constant
> unless its one of the above or part of an enum.


Yes. But that is a runtime error instead of a compile time error.

Moving errors from runtime to compile time is what type safety
is all about.

And many people (but not all) consider doing that a good thing.

Arne
Joerg Meier (10-26-18, 12:46 PM)
On Fri, 26 Oct 2018 00:37:10 +0000 (UTC), Martin Gregorie wrote:

>> And enum is type safe while int constants are not.

> AFAIK that is not relevant if the constant is defined as static final.


martinsMethod(BoxLayout.X_AXIS); // what now ?

The above may work, or not work, or change what it does between Java
versions, and will not be detected during compile time. I know it looks too
obvious to get wrong, and in this example it is, but once you start having
people copy and paste code around and extract parameters to variables and
fields, it becomes less obvious that there is a problem here.

If you feel the above example is too contrived:

foo(boolean bigDesign) {
int designSetting1;
int designSetting2;
if (bigDesign) {
designSetting1 = BoxLayout.X_AXIS;
designSetting2 = MartinsClass.LONG;
} else {
designSetting1 = BoxLayout.Y_AXIS;
designSetting2 = MartinsClass.SHORT;
}

// ...
// ...
// ...
// ...
// ...
// ...

doSomething(martinsClass(designSetting1));
}

Liebe Gruesse,
Joerg
Joerg Meier (10-26-18, 12:52 PM)
On Thu, 25 Oct 2018 15:53:04 -0400, Rhino wrote:

> Or did you have something different in mind?


Basically, that is what I had in mind, although two improvements:

> LocalDateTime ldt = LocalDateTime.now();
> ZonedDateTime zdt = ZonedDateTime.of(ldt, ZoneId.systemDefault());


LocalDateTime.now() is already returned in the system default time zone. So
you could rephrase this:

> String fullMonthName =
> CurrentDateTimeUtils.getCurrentMonth().getDisplayN ame(


as:

> LocalDateTime.now().getMonth().getDisplayName(


which aside of saving you from having a method is actually even shorter
than calling the method ;)

Liebe Gruesse,
Joerg
Eric Sosman (10-26-18, 01:50 PM)
On 10/25/2018 8:37 PM, Martin Gregorie wrote:
> On Thu, 25 Oct 2018 20:08:54 -0400, Arne Vajhj wrote:
> Sure, though being defined outside a class that is the only place its
> used (apart from when its supplied as a method argument) as we've been
> discussing does separate each constant definition and its documentation
> from the class definition and documentation


public class Whatnot {
public static enum Style { LONG, SHORT };
...
String method(Style style) { ... }
}

Whatnot what = new Whatnot();
String thing = what.method(Whatnot.Style.SHORT);

>> And enum will be better/easier documented for method arguments.

> Apart from the separation mentioned above. I see this as a disadvantage
> though you may not.


"Better/easier" is a *dis*advantage? Is this Opposite Day?

>> And enum is type safe while int constants are not.

> AFAIK that is not relevant if the constant is defined as static final.


Both Arne's and Martin's comments elude my understanding, so
I won't try to respond.

>> will be accepted by the compiler but have undefined semantics.

> Agreed, but being basically a mildly paranoid C programmer I tend to
> validate all arguments, including those created with a #define (C) or as
> static final int (Java) and you really shouldn't be defining a constant
> unless its one of the above or part of an enum.


Early mistake detection beats late misteak detectoin, every time.
Compile-time detection beats run-time detection.

> So far, it all looks a bit like swings and roundabouts to me except, of
> course, if your enum contains logic rather than being simply a list of
> constants.


My scorecard shows Arne with three, Martin with nil, and one
undecided.

Similar Threads