2
\$\begingroup\$

We are writing program which makes XML file which is a calendar with all free days in Poland (Saturdays, Sundays, and all hollidays). We tried to write this code as readable as possible so probably no comments are needed. What do You think about this code? What do You think is better in multiple files or maybe with Built-In Classes. And one more... which one is better and why?

in two lines

XMLInterior xmlInterior = new XMLInterior(whatYear);
Document document = xmlInterior.newDocumentXML();

or in one line

Document document =new XMLInterior(whatYear).newDocumentXML();

Here is whole code:

Main.java

public class Main {
    public static void main(String[] argc) throws Exception {
        int year = 2009;
        WhatFirstDayOfYear whatFirstDayOfYear = new WhatFirstDayOfYear(year);
        System.out.println(whatFirstDayOfYear.calculateWhatFirstDayOfYear());
        XMLMaker a = new XMLMaker(year);
    }
}

EasterCalculations.java

class EasterCalculations {
    int calculateEasterDate(int year, boolean isLeapYear) {
        final int centuryPaschalFullMoon = 24, cycleOfDaysOfWeek = 5; //valid until 2099 A.D.
        int cycleMetonic, leapJulian, nonLeapYear, datePaschalFullMoon, firstSundayAfterPaschalFullMoon;

        cycleMetonic = year % 19;
        leapJulian = year % 4;
        nonLeapYear = year % 7;

        datePaschalFullMoon = (cycleMetonic * 19 + centuryPaschalFullMoon) % 30;
        firstSundayAfterPaschalFullMoon = (2 * leapJulian + 4 * nonLeapYear + 6 * datePaschalFullMoon + cycleOfDaysOfWeek) % 7;

        if (29 == datePaschalFullMoon && 6 == firstSundayAfterPaschalFullMoon)
            return isLeapYear ? 31 + 29 + 31 + 19 : 31 + 28 + 31 + 19;
        else if (28 == datePaschalFullMoon && 6 == firstSundayAfterPaschalFullMoon && cycleMetonic > 10)
            return isLeapYear ? 31 + 29 + 22 + datePaschalFullMoon + firstSundayAfterPaschalFullMoon - 7 : 31 + 28 + 22 + datePaschalFullMoon + firstSundayAfterPaschalFullMoon - 7;
        return isLeapYear ? 31 + 29 + 22 + datePaschalFullMoon + firstSundayAfterPaschalFullMoon : 31 + 28 + 22 + datePaschalFullMoon + firstSundayAfterPaschalFullMoon;
    }
}

WhatFirstDayOfYear.java

class WhatFirstDayOfYear {
    private int whatYear;
    WhatFirstDayOfYear(int whatYear) {
        this.whatYear = whatYear;
    }
    int calculateWhatFirstDayOfYear(){
        int tempYear = 2019;
        int tempYearFirstDay = 2;
        if(whatYear > tempYear){
            while(whatYear != tempYear){
                if(tempYear % 4 == 0 && tempYear % 100 != 0 || tempYear % 400 == 0) tempYearFirstDay += 2;
                else tempYearFirstDay++;
                tempYear++;
            }
        }
        else if(whatYear < tempYear){
            while(whatYear != tempYear){
                if(whatYear % 4 == 0 && whatYear % 100 != 0 || whatYear % 400 == 0) tempYearFirstDay -= 2;
                else tempYearFirstDay--;
                whatYear++;
            }
        }
        else return  tempYearFirstDay;
        return (tempYearFirstDay%7+7)%7;
    }
}

XMLInterior.java

import org.w3c.dom.Attr;
import org.w3c.dom.Document;
import org.w3c.dom.Element;

import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;

class XMLInterior {
    private Document document;
    private Element element;
    private Element month;
    private Attr holiday;
    private int startYearDay;
    private int currentYearDay;
    private int dateEaster;
    private int dateCorpusCristi;
    private int datePentecost;
    private boolean isLeapYear;

    XMLInterior(int whatYear) {
        EasterCalculations easterCalculations = new EasterCalculations();
        WhatFirstDayOfYear whatFirstDayOfYear = new WhatFirstDayOfYear(whatYear);
        this.startYearDay = whatFirstDayOfYear.calculateWhatFirstDayOfYear();
        this.currentYearDay = 1;
        isLeapYear = whatYear % 4 == 0 && whatYear % 100 != 0 || whatYear % 400 == 0;
        dateEaster = easterCalculations.calculateEasterDate(whatYear, isLeapYear);
        dateCorpusCristi = dateEaster + 40;
        datePentecost = dateEaster + 60;
    }

    Document newDocumentXML() throws Exception {
        DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
        DocumentBuilder documentBuilder = documentBuilderFactory.newDocumentBuilder();

        document = documentBuilder.newDocument();

        element = document.createElement("Year");
        document.appendChild(element);
        makeYear();

        return document;
    }

    private void createMonthInterior(int numberMonth, int numberDays) {
        int dayOfMonth = 1;
        Attr noOfDay;
        Element day;
        for (int i = 1; i <= numberDays; i++) {
            day = document.createElement("Day");
            holiday = document.createAttribute("Holiday");
            checkHoliday(numberMonth, dayOfMonth);
            day.setAttributeNode(holiday);
            noOfDay = document.createAttribute("NoDay");
            noOfDay.setValue(Integer.toString(i));
            day.setAttributeNode(noOfDay);
            month.appendChild(day);
            startYearDay++;
            currentYearDay++;
            dayOfMonth++;
        }
    }

    private void createMonth(String name, int numberDays, int monthNumber) {
        month = document.createElement(name);
        element.appendChild(month);
        Attr noDays = document.createAttribute("NoDays");
        noDays.setValue(Integer.toString(numberDays));
        month.setAttributeNode(noDays);
        Attr noMonth = document.createAttribute("NoMonth");
        noMonth.setValue(Integer.toString(monthNumber));
        month.setAttributeNode(noMonth);
        createMonthInterior(monthNumber, numberDays);
    }

    private void makeYear() {//enum
        createMonth("January", 31, 1);
        if (isLeapYear)
            createMonth("February", 29, 2);
        else
            createMonth("February", 28, 2);
        createMonth("March", 31, 3);
        createMonth("April", 30, 4);
        createMonth("May", 31, 5);
        createMonth("June", 30, 6);
        createMonth("July", 31, 7);
        createMonth("August", 31, 8);
        createMonth("September", 30, 9);
        createMonth("October", 31, 10);
        createMonth("November", 30, 11);
        createMonth("December", 31, 12);
    }

    private void checkHoliday(int noMonth, int currentMonthDay) {
        if (6 == startYearDay % 7 || 0 == startYearDay % 7) holiday.setValue("1");
        else if (1 == noMonth && (1 == currentMonthDay || 6 == currentMonthDay)) holiday.setValue("1");
        else if (5 == noMonth && (1 == currentMonthDay || 3 == currentMonthDay)) holiday.setValue("1");
        else if (8 == noMonth && 15 == currentMonthDay) holiday.setValue("1");
        else if (11 == noMonth && (11 == currentMonthDay || 1 == currentMonthDay)) holiday.setValue("1");
        else if (12 == noMonth && (25 == currentMonthDay || 26 == currentMonthDay)) holiday.setValue("1");
        else if (currentYearDay == dateEaster || currentYearDay == dateEaster + 1) holiday.setValue("1");
        else if (currentYearDay == dateCorpusCristi) holiday.setValue("1");
        else if (currentYearDay == datePentecost) holiday.setValue("1");
        else holiday.setValue("0");
    }
}

XMLMaker.java

import org.w3c.dom.Document;

import javax.xml.transform.Transformer;
import javax.xml.transform.TransformerFactory;
import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;
import java.io.File;

class XMLMaker {
    XMLMaker( int whatYear) throws Exception {
        makeXML( whatYear);
    }

    private void makeXML( int whatYear) throws Exception {
        XMLInterior xmlInterior = new XMLInterior(whatYear);
        Document document = xmlInterior.newDocumentXML();

        TransformerFactory transformerFactory = TransformerFactory.newInstance();
        Transformer transformer = transformerFactory.newTransformer();
        DOMSource src = new DOMSource(document);

        String pathName = "C:\\XML";
        checkDirectory(pathName);
        StreamResult streamResult = new StreamResult(new File(pathName + "\\" + whatYear + ".xml"));
        transformer.transform(src, streamResult);
    }

    private void checkDirectory(String pathName)
    {
        File directoryXML = new File(pathName);
        if (!directoryXML.exists())
            if (!directoryXML.mkdir())
                System.out.println("Can't mkdir directory");
        if (!directoryXML.isDirectory())
            System.exit(1);
    }
}
\$\endgroup\$

1 Answer 1

0
\$\begingroup\$

Easter: Leap Year Check Is Redundant

Given that it is easy to calculate whether a year is a leap year, any performance loss from re-calculating it will be miniscule compared to the risk of the two being inconsistent.

Easter: DRY (Don't Repeat Yourself)

There are several places which, essentially, amount to this:

return isLeapYear ? (stuff) + 1 : (stuff)

It would reduce the amount of repetition by calculating as if it weren't a leap year, then adding an extra day if necessary.

What First Day Of Year?

Isn't this always January 1st? I can't work out what it is calculating. Consider renaming it.

XMLInterior: Revisit Your Fields

Fields are meant to be used across calls to various methods of an object. In your case, most of the fields set by newDocumentXML won't be accessible. These should be passed around as parameters and return values instead.

XMLInterior: What are these holidays?

It would make this code a lot clearer to have methods that tested each holiday, such as isNewYearsDay(date).

XMLMarker: Consider using NIO methods for creating directories

Methods in java.nio.Files throw exceptions on failure, rather than returning a boolean to indicate success or not. These will make the code significantly cleaner.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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