4
\$\begingroup\$

I created the following program that transforms a numerical month into the corresponding textual month. I initially calculated the starPoint and endPoint in the class constructor; then I changed that calculation to the getName method. Which is a better option? Calculating in the class constructor, or in a separate method like shown in the code?

/**
 * Created by Robert Holland on 25/11/2016.
 *
 * A class that accepts a number and calculates
 * the corresponding month. Once the month has
 * been calculated it will then be returned.
 *
 */

public class Month
{
    public static final String MONTHS = "January   " + "February  " + "March     " +
                                        "April     " + "May       " + "June      " +
                                        "July      " + "August    " + "September " +
                                        "October   " + "November  " + "December  ";
    private int monthNumber;

    //Construct a month class that has a value equal to the numerical month
    //@param uMonthNumber the number of the month to find the correspondance.
    public Month(int uMonthNumber)
    {
        monthNumber = uMonthNumber;
    }

    //Determine the name that corresponds to the month number.
    //@return month the name of the corresponding month.
    public String getName()
    {
        String month;
        int startPoint;
        int endPoint;

        endPoint = monthNumber * 10;
        startPoint = endPoint - 10;
        month = MONTHS.substring(startPoint, endPoint);

        return month;
    }
}
\$\endgroup\$
3
\$\begingroup\$

This exists already:

It's not the prettiest to use but it's there.

import java.time.*;
import java.time.format.*;
import java.util.*;

public class Example {

    public static void main(String[] args) {
        Month month = Month.of(2);
        System.out.printf("%s", month.getDisplayName(TextStyle.FULL, new Locale("en")));
        // outputs:
        // February
    }
}

Don't re-write code that's in your standard libraries :)

\$\endgroup\$
  • 1
    \$\begingroup\$ Month.of(2); is also Month.FEBRUARY, which is prettier. Also new Locale("en") could be replaced with Locale.ENGLISH. Avoids creating objects (since it reuses a constant), and is more descriptive. \$\endgroup\$ – Tunaki Nov 25 '16 at 19:38
2
\$\begingroup\$

It doesn't make sense why you're putting all 12 strings into a single String rather than an array.

You may also wish to return month.trim() to get rid of the extra whitespace.

\$\endgroup\$
2
\$\begingroup\$

To short as an answer so leaving as a comment: Why not make getName() the toString() for your class? Also, concatenating 12 strings together only to keep applying String.substring(int, int) every time you want to get the name sounds a little counterintuitive IMHO...

(from my comment)

The main advantage of using toString() is so that the textual representation is available for use by users of your class easily.

Getting to the crux of your question (which I have also somewhat answered above):

Which is a better option? Calculating in the class constructor, or in a separate method like shown in the code?

My advice is that if there's a variable on your class that should be immutable the moment an object is instantiated, it should be done in the class constructor. That's because you can declare the variable as final and you can reliably use it within your class knowing that it cannot be changed. Your Month class seems like a good example of this. :)

A suggestion (using your coding style):

public class Month
{
    private final int monthNumber;
    private final String monthName;

    public Month(int monthNumber)
    {
        this.monthNumber = monthNumber;
        this.monthName = generateName(monthNumber);
    }

    @Override
    public String toString()
    {
        return monthName;
    }

    private static generateName(int monthNumber)
    {
        // ...
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.