HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Days of life calculator for first-year mathematics students

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
yearmathematicsstudentsfirstforlifecalculatordays

Problem

I'm giving a lecture on basic programming to first-year mathematics students. The programming language is Java (defined by curriculum).

Last week I gave the following task:


Write a program, that reads in the date of birth of the user separated into year, month and day and subsequently calculates the days the user has lived until today.

Do consider leap years.

This task seems not to be too hard, but there are some restrictions (as they are first-year students and never had had any experiences in programming):

  • only the basic data types are known (int, double, boolean, String etc.)



  • arrays are not known



  • writing own functions is not known (i.e. everything is within the main)



  • except of System.out.print/ln none of the Java API functions are known



  • user input is realised through a provided method readInteger(String promt) (the students just use it, but don't know why and how it works)



Here I present my sample solution and would like to know, whether there are some more possibilities of "smoothing" the code, optimising it or just increasing the readability. Only tweaks and tricks are allowed, which can be done given the knowledge-restrictions above.

```
public static void main(String[] args) {
// define today's day
final int TODAY_YEAR = 2011, TODAY_MONTH = 4, TODAY_DAY = 29;
// define a leapyear in the past
final int LEAPYEAR = 1904;
// is the current year a leapyear?
boolean todayIsLeapyear = false;

// get user input
int birthYear = readInteger("Please enter the year of your birth:");
int birthMonth = readInteger("Please enter the month of your birth:");
int birthDay = readInteger("Please enter the day of your birth:");

// check input for validity
boolean inputOK = true;
int birthYearLeapShift = 0;
boolean birthYearWasLeap = false;
if (birthYear > 2011 || birthMonth > 12) {
System.out.println("Your input was not correct.");
if (birthYear > 2011) {

Solution

The code contains a number of things that would be considered bad style ... and you should not be requiring students to learn/practice bad style. Some of the big problems are:

-
A main method that is 100's of line long is in dire need of refactoring.

-
Drop through in switch statements is considered to be bad style. Certainly, it is rarely used in real applications. You've done it 2 or 3 times.

-
Hard wiring today's date into a program is a terrible idea. Teaching students to hard wire environmental information is a really bad idea.

I'd say that the task that you have set is way too complicated given the restrictions. At the very least, students need to understand static methods and method calls to undertake a task like this. They also need to be told about System.currentTimeMillis().

Given that the problem is set in concrete, and the students' knowledge is as stated, there's not much you can do to improve the readability of the solution, I'm afraid.

The best you could do would be to show them what the ideal solution would look like ... if they were using a larger subset of Java. (And then maybe "fess up" to the fact that the problem was too hard.)

Drop-through switch cases are bad style because drop-through is easy to miss. (And in fact, it is usually an error.) If you really want to use it, you should have a comment at each point where drop through is intended to occur; see http://www.oracle.com/technetwork/java/codeconventions-142311.html#468

I will grant you that yours is an elegant solution ... but a more elegant one would be to use array lookup; e.g.

final static int[] DAYS_TO_START_OF_MONTH = {
                // Line them up neatly so that it is trivial to manually inspect
                // the constant expressions ...
                0,
                31,
                31 + 28,
                31 + 28 + 31,
                ...
    };

    public int daysToStartOfMonth(int month, boolean isLeapYear) {
          return DAYS_TO_START_OF_MONTH[month] + (month >= 2 && isLeapYear ? 1 : 0);
    }


Teaching people bad programming style is a bad idea period. A lot of students who graduate with a mathematics degree do end up in programming jobs ... and need remedial mentoring to help them unlearn their bad programming habits.

Code Snippets

final static int[] DAYS_TO_START_OF_MONTH = {
                // Line them up neatly so that it is trivial to manually inspect
                // the constant expressions ...
                0,
                31,
                31 + 28,
                31 + 28 + 31,
                ...
    };

    public int daysToStartOfMonth(int month, boolean isLeapYear) {
          return DAYS_TO_START_OF_MONTH[month] + (month >= 2 && isLeapYear ? 1 : 0);
    }

Context

StackExchange Code Review Q#2161, answer score: 8

Revisions (0)

No revisions yet.