4
\$\begingroup\$

I got bored on my long weekend so I decided to have my coding exercise. And I successfully made a program that let you input the day and the month then it gives you the day of given date. But sadly it's only for 2014 calendar.

import java.io.*;

public class DaysToYears{
public static void main(String[] args) throws ArrayIndexOutOfBoundsException{
  BufferedReader dataIn = new BufferedReader(new InputStreamReader(System.in));
  try{
    String []yearName = {"January","February","March","April","May","June","July","August","September","October","November","December"};
    String []days = {"Sunday","Monday","Tuesday","Wednesday","Thursday","Friday","Saturday"};
    int []yearsCode = {6, 2, 2, 5, 0, 3, 5, 1, 4, 6, 2, 4};
    int firstCase = 0;
    int secondCase = 0;
    String inputMonth = new String();
    int inputDate = 0;
      System.out.print("Enter your month: ");
        inputMonth = dataIn.readLine();
      System.out.print("Enter your date:  ");
        inputDate = Integer.parseInt(dataIn.readLine());
          for(int nodeCount=0;nodeCount<yearName.length;nodeCount++){
              if(inputMonth.equalsIgnoreCase(yearName[nodeCount])){
                if(inputDate < 7){
                    firstCase = (yearsCode[nodeCount] + inputDate + 3) % 7;
                    System.out.println(yearName[nodeCount] + " " + inputDate + " is " + days[firstCase]);
                    }else{
                      secondCase = (yearsCode[nodeCount] + inputDate + 3) % 7;
                      System.out.println(yearName[nodeCount] + " " + inputDate + " is " + days[secondCase]);
                      }
                    }
                  }
    }catch(IOException e){
      System.out.println("Error getting input!");
    }
  }
}
\$\endgroup\$
  • 3
    \$\begingroup\$ See Zeller's Congruence, and my attempt here \$\endgroup\$ – rolfl Dec 9 '14 at 11:39
  • \$\begingroup\$ I saw your code earlier. It's nice. Another algorithm aside from Doomsday Algorithm. I'll try to implement Zeller's Congruence if I got time. Thanks :) \$\endgroup\$ – Yodism Dec 9 '14 at 11:47
2
\$\begingroup\$

Outline

Why is the class called DaysToYears? Shouldn't it be called something like Calendar2014?

A main() that is expected to throw ArrayIndexOutOfBoundsException is sloppy. The equivalent in C would be a crash from a segmentation fault. Here, it means that the user entered an unrecognized month name, and you haven't handled the invalid input properly.

There should be a method

public static String dayOfWeek(String month, int day) throws IllegalArgumentException

The constants should be static final members of the class, and they should be named in ALL_CAPS by convention.

In summary, the outline should look like

public class Calendar2014 {
    // Using a List<String> instead of a String[] to take advantage of
    // List.indexOf()
    private static final List<String> MONTHS = Arrays.asList(new String[] {
        "January", "February", "March", "April", "May", "June",
        "July", "August", "September", "October", "November", "December"
    });


    private static final String[] DAYS = {
        "Sunday", "Monday", …, "Saturday"
    };

    // Replacement for yearsCode.  To be discussed below.
    private static final int[] MONTH_START_DAYS = { … };

    public static String dayOfWeek(String month, int dayOfMonth) throws IllegalArgumentException {
        …
    }

    public static void main(String[] args) {
        BufferedReader dataIn = new BufferedReader(new InputStreamReader(System.in));
        try {
            …
        } catch (IOException e) {
            // Errors should go to System.err, not System.out
            System.err.println("Error getting input!");
        }
    }
}

Input processing

To find convert the month name to a number, use List.indexOf().

nodeCount is not a useful variable name. I'd call it month.

    public static String dayOfWeek(String monthName, int dayOfMonth) throws IllegalArgumentException {
        int month = MONTHS.indexOf(monthName);
        if (month < 0) {
            throw new IllegalArgumentException("Invalid month name: " + monthName);
        }
        …
    }

Algorithm

I see no reason for splitting the handling into two cases. The two branches are identical!

yearsCode is poorly designed. Why not add 3 to every element, so that you don't have to do it here

(yearsCode[nodeCount] + inputDate + 3) % 7

Then, the array represents the day of week of "day 0" of the month. So, let's rename it to MONTH_START_DAYS.

    private static final int[] MONTH_START_DAYS = {
        2, 5, 5, 1, 3, 6, 1, 4, 0, 2, 5, 0
    };

    public static String dayOfWeek(String monthName, int dayOfMonth) throws IllegalArgumentException {
        int month = Arrays.binarySearch(MONTHS, monthName);
        if (month < 0) {
            throw new IllegalArgumentException("Invalid month name: " + monthName);
        }
        return DAYS[(MONTH_START_DAYS[month] + dayOfMonth) % 7];
    }

Suggested solution

import java.io.*;
import java.util.Arrays;
import java.util.List;

public class Calendar2014 {
    private static final List<String> MONTHS = Arrays.asList(new String[] {
        "January", "February", "March", "April", "May", "June",
        "July", "August", "September", "October", "November", "December"
    });

    private static final String[] DAYS = {
        "Sunday", "Monday", "Tuesday", "Wednesday",
        "Thursday", "Friday", "Saturday"
    };

    /** Day of week of "day 0" of each month */
    private static final int[] MONTH_START_DAYS = {
        2, 5, 5, 1, 3, 6, 1, 4, 0, 2, 5, 0
    };

    public static String dayOfWeek(String monthName, int dayOfMonth) throws IllegalArgumentException {
        int month = MONTHS.indexOf(monthName);
        if (month < 0) {
            throw new IllegalArgumentException("Invalid month name: " + monthName);
        }
        return DAYS[(MONTH_START_DAYS[month] + dayOfMonth) % 7];
    }

    public static void main(String[] args) {
        BufferedReader dataIn = new BufferedReader(new InputStreamReader(System.in));
        try {
            System.out.print("Enter your month: ");
            String month = dataIn.readLine();
            System.out.print("Enter your date: ");
            int date = Integer.parseInt(dataIn.readLine());
            System.out.printf("%s %d is %s\n", month, date,
                                               dayOfWeek(month, date));
        } catch (IOException e) {
            // Errors should go to System.err, not System.out
            System.err.println("Error getting input!");
        } catch (IllegalArgumentException badDate) {
            System.err.println("Invalid date");
        }
    }
}
\$\endgroup\$
2
\$\begingroup\$

This "new String()" is unnecessary:

String inputMonth = new String();

I would leave it out completely:

String inputMonth;
\$\endgroup\$
  • \$\begingroup\$ Why? But I used the equalsIgnoreCase statement. Is it okay? \$\endgroup\$ – Yodism Dec 9 '14 at 13:46
  • 1
    \$\begingroup\$ New String() assigns a value to inputMonth, but it is not used. A few lines later you assign a new value to the same variable: "inputMonth = dataIn.readLine();". So before you used inputMonth you assigned a new value to it. equalsIgnoreCase statement is ok, I see no problem with that. \$\endgroup\$ – helppo Dec 9 '14 at 13:59
2
\$\begingroup\$

You're mixing input, computation, and output in one method. This is bad, as you can't use it anywhere else. All you have is a tool requiring user interaction, nothing what can be used from another code. Luckily, all you need here is to reorder the lines a bit and then "extract method". Your IDE will do it for you.


String inputMonth = new String();

As already said, this is unnecessary. Moreover, new String() is always wrong as it creates a new String, but all you could even need is

String inputMonth = "";

String inputMonth = new String();
int inputDate = 0;
  System.out.print("Enter your month: ");
    inputMonth = dataIn.readLine();
  System.out.print("Enter your date:  ");
    inputDate = Integer.parseInt(dataIn.readLine());

Fix indentation and move the declaration to initialization like

System.out.print("Enter your month: ");
String inputMonth = dataIn.readLine();
System.out.print("Enter your date:  ");
int inputDate = Integer.parseInt(dataIn.readLine());

to save lines and make it clearer.


}catch(IOException e){
  System.out.println("Error getting input!");
}

Imagine a 100k lines program you got to fix, which prints "Error getting input!" from time to time. Sure, you can search for the string (and find some hundreds of occurrences), but what about

} catch (IOException e) {
     e.printStackTrace()
}

(note the spacing) which would tell you immediately exactly what happened and where (and it'd go to stderr rather than stdout, which is better). Actually, in main, I never do this, simply

public static void main(String[] args) throws IOException {
    ...
}

does the job perfectly. Only handle exceptions if you can do something meaningful, otherwise let them propagate (or wrap them if needed). That's the simplest and best solution, just let it be.

Imagine java.io.* would "handle" the exception the way you do... you'd get some terminal output, but your program would know nothing about the problem.

\$\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.