2

I'm a beginner i have an assignment to write a Date class (as part of a bigger project). in my question i focus on the constructor. here's some background: the given guidelines are that the date is not expected to be valid and the following instance variables expect this input range: day- integer 1-31 month- integer 1-12 year- integer 4 digits year.

now, if an invalid day/month/year or invalid date (such as 31.2.2010) is entered, the object will be created with the date of 1.1.2000.

this is the code I've come up with and it does compile and seem to work fine.

public class Date
{
private int _day;
private int _month;
private int _year;


public Date (int day, int month, int year)  
{
    switch (month)
    {
        case 1:                
        case 3:
        case 5:
        case 7:
        case 8:
        case 10:
        case 12: if ((day>0 && day<32) && (year>999 && year<10000))
        {
            _day=day;
            _month=month;
            _year=year;           
        }
        else
        {
            _day=1;
            _month=1;
            _year=2000;
        }
        break;

        case 4:
        case 6:
        case 9:
        case 11: if ((day>0 && day<31) && (year>999 && year<10000))
        {
            _day=day;
            _month=month;
            _year=year;           
        }
        else
        {
            _day=1;
            _month=1;
            _year=2000;
        }
        break;

        case 2: if (leap(year))
        {
            if ((day>0 && day<30) && (year>999 && year<10000))
            {
                _day=day;
                _month=month;
                _year=year;           
            }
            else
            {
                _day=1;
                _month=1;
                _year=2000;
            }
            break;
        }

        else
        {
            if ((day>0 && day<29) && (year>999 && year<10000))
            {
                _day=day;
                _month=month;
                _year=year;           
            }
            else
            {
                _day=1;
                _month=1;
                _year=2000;
            }
            break;
        }
    }
}

/** check if leap year */
private boolean leap (int y)  
{
    return (y % 4 == 0 && y % 100 != 0) || (y % 400 == 0);
}

}

here are my questions:

  1. is it fine to put all that code in the constructor? will it greatly affect the processing time or cause an error? is there an alternative if its a problem?
  2. is any part of the code could be considered a bad practice? such as the switches and ifs? I'm not feeling to confident with this build despite it working fine...
  • 4
    Better suited for https://codereview.stackexchange.com/ – Thiyagu Mar 22 '18 at 17:39
  • 3
    It is not good style to call variables '_day', '_month', etc. Later when you are setting '_day = day'. Use 'this.day = day'. The 'this' identifier refers to the class itself. – mstill3 Mar 22 '18 at 17:44
  • 2
    Nothing wrong with much code in constructor (a more modern approach would have a static factory method, but you probably don’t need to care). You seem to repeat a lot with small variations. That’s usually a sign that the code can be improved. I suggest a method that returns the number of days in a given month, then you need to have your validation code only once. And/or I suggest a `setToDefault` method that will set the date to 1.1.2000, so you don’t need to have code for that four times. The principle is [DRY for don’t repeat yourself](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself). – Ole V.V. Mar 22 '18 at 18:02
  • @OleV.V. thanks for the answer. as of now, we are very limited in the code we can write. the professors expect a sort of strict way of programming-they have given us a set amount of methods and their addresses, and we need to fil the rest. later on i do believe and hope we'll have more programming freedom, and then ill be taking to heart your suggestions. – Eric Parker Mar 22 '18 at 18:06

1 Answers1

0

is it fine to put all that code in the constructor? will it greatly affect the processing time or cause an error? is there an alternative if its a problem?

Well, there's nothing wrong with the code in your constructor body. As long as your syntax is correct, your code will always run fine. However, one important thing you need to know is this: When the complexity of the code in the body of your constructor reaches a certain level, it starts to tell on your processing time. A good illustration is a situation where you have multiple loops and/or heavy background processes in your constructor. This is the reason why putting heavy processes in a constructor body is frowned upon (but not prohibited).

is any part of the code could be considered a bad practice?

There are a number of ways you can optimize your code:

  1. You can give your global and instance variables the same name and distinguish between them using the this keyword.. i.e: this.day = day;

  2. Take advantage of already-available classes and packages in Java. The Calendar class has methods that return the number of days in a month and so on. Instead of reinventing the wheel, use them. Check this question to see how to get the number of days in a month. There are bunch of other methods that may be useful to you too.

  3. Lastly, You can declare a separate method to set the value to your chosen default state (01-01-2000) so that you won't have to be setting it every time, this would largely reduce your lines of code.

I hope this helps.. Merry coding!

Taslim Oseni
  • 6,086
  • 10
  • 44
  • 69