6
\$\begingroup\$

I maintain our organizations intranet page as an additional duty. One thing that every department in the organization likes to use is calendars -- upcoming events, training, etc. At first I was only getting a few a month, and it wasn't difficult to create the calendars using cut-n-paste. I wrote the following to create a generic calendar as the volume of requests has gone through the roof recently.
I'm also the only programmer in the organization and don't have anybody to review code. I would appreciate any comments, suggestions, or critiques.

private void jButton1ActionPerformed(java.awt.event.ActionEvent evt) {                                         
    GregorianCalendar gc = new GregorianCalendar();
    int month = jComboBox1.getSelectedIndex();
    int year = Integer.parseInt((String) jComboBox2.getSelectedItem());

    gc.set(Calendar.DAY_OF_MONTH, 1);
    gc.set(Calendar.MONTH, month);
    gc.set(Calendar.YEAR, year);
    createCalendar(gc);
}

//receives GregorianCalendar with date set to first day of appropriate month and year.
private void createCalendar(GregorianCalendar gc) {
    FileWriter fw = null;
    SimpleDateFormat sdf = new SimpleDateFormat("MMM");
    int startDayOfWeek = gc.get(Calendar.DAY_OF_WEEK);
    int dayCounter = 1;

    //create table, set calendar "look"
    String monthHTML = "                <table style=\"width:100%;border-collapse: collapse;border: thin green solid;background-color:#E0F0FF;\">\n"
            + "                    <tr style=\"width:100%;border: thin black solid;\">\n"
            + "                        <td style=\"width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center\">&nbsp;S</td>\n"
            + "                        <td style=\"width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center\">&nbsp;M</td>\n"
            + "                        <td style=\"width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center\">&nbsp;T</td>\n"
            + "                        <td style=\"width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center\">&nbsp;W</td>\n"
            + "                        <td style=\"width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center\">&nbsp;T</td>\n"
            + "                        <td style=\"width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center\">&nbsp;F</td>\n"
            + "                        <td style=\"width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center\">&nbsp;S</td>\n"
            + "                    </tr>\n"
            + "                    <tr style=\"width:100%;border: thin black solid;height:100px\">\n";

    //add leading "blank" days for Sun-Sat format and count
    for (int x = 1; x < startDayOfWeek; x++) {
        monthHTML += "                        <td style=\"width:12%;border: none;margin: auto;text-align:center\">&nbsp;&nbsp;</td>\n";
    }
    //loop through rest of first week
    int daysRemainingInWeek = 7 - startDayOfWeek;
    for (int y = 0; y < daysRemainingInWeek + 1; y++) {
        monthHTML += "                        <td style=\"width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;\">\n"
                + "                            <div style=\"height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;\">" + dayCounter + "</div>\n"
                + "                            <div style=\"width: 100%;text-align: left;\">\n"
                + "                                <ul>\n"
                + "                                </ul>\n"
                + "                            </div>\n"
                + "                        </td>\n";
        dayCounter++;
    }
    monthHTML += "                    </tr>\n"
            + "                    <tr style=\"width:100%;border: thin black solid;height:100px\">\n";

    //get sentinal for loop for rest of calendar, add one for zero-based
    int monthLength = gc.getActualMaximum(Calendar.DAY_OF_MONTH) + 1;


    for (int z = dayCounter; z < monthLength; z++) {
        //determine/set line breaks.  Still trying to figure out why it needs the "dayCounter - 2)" to work properly
        if ((startDayOfWeek + dayCounter - 2) % 7 == 0) {
            monthHTML += "                        </tr>\n"
                    + "                    <tr style=\"width:100%;border: thin black solid;height:100px\">\n";
        }
        monthHTML += "                        <td style=\"width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;\">\n"
                + "                            <div style=\"height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;\">" + dayCounter + "</div>\n"
                + "                            <div style=\"width: 100%;text-align: left;\">\n"
                + "                                <ul>\n"
                + "                                </ul>\n"
                + "                            </div>\n"
                + "                        </td>\n";
        dayCounter++;
    }
    monthHTML += "                    </tr>\n"
            + "                </table>\n";

    // hat tip to Tim Castelijns, http://stackoverflow.com/questions/14832151/how-to-get-month-name-from-calendar for file naming idea
    File file = new File("C:/temp/" + sdf.format(gc.getTime()) + gc.get(Calendar.YEAR) + ".html");
    try {
        fw = new FileWriter(file.getAbsoluteFile());
        BufferedWriter bw = new BufferedWriter(fw);
        bw.write(monthHTML);
        bw.close();
        fw.close();
    } catch (IOException ioe) {
        ioe.printStackTrace();
    }
}
\$\endgroup\$
  • \$\begingroup\$ What, if anything, is the generated file used for? \$\endgroup\$ – 200_success Feb 11 '16 at 19:05
  • \$\begingroup\$ They're used to generate blank calendars that I can paste into a department's page on our intranet. It still needs to be updated with their specific information. \$\endgroup\$ – Bob Stout Feb 11 '16 at 19:09
4
\$\begingroup\$

There are a couple things that could be improved with your code:

  • You are not correctly closing the BufferedWriter, resulting in a potential resource leak (this happens when an exception is raised). To tackle this, you could use a try-with-resources statement:

    try (BufferedWriter bw = new BufferedWriter(new FileWriter(file.getAbsoluteFile()))) {
        bw.write(monthHTML);
    } catch (IOException ioe) {
        ioe.printStackTrace();
    }
    
  • You are building a long string by appending each part with += when it would be more performant to use a StringBuilder.

  • Consider giving a Calendar instead of a GregorianCalendar as input.

The biggest concern I have is that your code is not very maintanable because it couples too much your data with the way you want to print it. As such, it is not very flexible: what is tomorrow you need to add another column? You would need to modify that tricky HTML code, which is error-prone.

What I would do instead is try to decouple the data from its presentation by using a templating framework. There are a lot out there, but I will continue by giving an example with Apache Velocity.

The logic now becomes:

  • Create a template of the page you want. You can use construct like foreach loop or conditionals.
  • Ask the templating framework to build your page by merging your data to your template.

As a simple example, with Velocity, you could for example have the following:

#foreach( $dayCounter in [1..5] )
<td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
    <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">$dayCounter</div>
    <div style="width: 100%;text-align: left;">
        <ul>
        </ul>
    </div>
</td>
#end

Then, with the following code:

Velocity.init();
VelocityContext context = new VelocityContext();
// add some property to context like: context.put("property", "Velocity");
Template template = Velocity.getTemplate("mytemplate.vm");
try (BufferedWriter bw = new BufferedWriter(...)) {
    template.merge(context, bw);
}

Velocity will directly merge your business data into your tempate and produce your wanted file.

Of course, you could use another templating engine: the logic is the same. As such, no code is duplicated, the HTML code is easily editable, even by someone who doesn't know the engine... I think you could benefit a lot from that.

\$\endgroup\$
  • \$\begingroup\$ and that's exactly the kind of stuff I was looking for. Thank you. \$\endgroup\$ – Bob Stout Feb 11 '16 at 20:32
3
\$\begingroup\$

Output quality

Your output for February 2016 looks like this:

                <table style="width:100%;border-collapse: collapse;border: thin green solid;background-color:#E0F0FF;">
                    <tr style="width:100%;border: thin black solid;">
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center">&nbsp;S</td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center">&nbsp;M</td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center">&nbsp;T</td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center">&nbsp;W</td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center">&nbsp;T</td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center">&nbsp;F</td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center">&nbsp;S</td>
                    </tr>
                    <tr style="width:100%;border: thin black solid;height:100px">
                        <td style="width:12%;border: none;margin: auto;text-align:center">&nbsp;&nbsp;</td>
                        <td style="width:12%;border: none;margin: auto;text-align:center">&nbsp;&nbsp;</td>
                        <td style="width:12%;border: none;margin: auto;text-align:center">&nbsp;&nbsp;</td>
                        <td style="width:12%;border: none;margin: auto;text-align:center">&nbsp;&nbsp;</td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">1</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">2</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">3</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                    </tr>
                    <tr style="width:100%;border: thin black solid;height:100px">
                        </tr>
                    <tr style="width:100%;border: thin black solid;height:100px">
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">4</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">5</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">6</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">7</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">8</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">9</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">10</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        </tr>
                    <tr style="width:100%;border: thin black solid;height:100px">
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">11</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">12</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">13</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">14</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">15</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">16</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">17</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        </tr>
                    <tr style="width:100%;border: thin black solid;height:100px">
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">18</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">19</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">20</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">21</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">22</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">23</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">24</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        </tr>
                    <tr style="width:100%;border: thin black solid;height:100px">
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">25</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">26</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">27</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">28</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                        <td style="width:12%;border: thin black solid;border-collapse: collapse;margin: auto;text-align:center;">
                            <div style="height: 10%;float:right;width:12%; color: #0033CC; background-color:lightblue;font-size: larger;border: thin black solid;">29</div>
                            <div style="width: 100%;text-align: left;">
                                <ul>
                                </ul>
                            </div>
                        </td>
                    </tr>
                </table>

Note that there is an extra <tr> tag after Feb 3, due to the monthHTML += "</tr><tr>" before //get sentinal for loop for rest of calendar.

The bigger issue is that writing style attributes everywhere, especially repetitive ones, is considered poor practice: styling should be done using stylesheets for compactness and maintainability. (The only exception is if you are writing HTML to be embedded in e-mail, because some e-mail clients have primitive HTML renderers.)

Did you mean for the numbers to appear in the upper-right corner instead of being centered vertically on the right? If so, you'll want a vertical-align: top rule. Furthermore, since the numbers are floated, you may want to put a clear: right rule on the <ul> elements so that they start consistently below the number. Avoid hard-coding a height and width for the number box that won't scale with the text size.

I would also put a <caption> on the table as its title. If you don't want to see it in the output, just put a display: none rule in the CSS to suppress it.

The output should look more like this:

table.calmonth {
    width: 100%;
    border-collapse: collapse;
    border: thin green solid;
    background-color: #E0F0FF;
}
table.calmonth td {
    height: 100px;
    vertical-align: top;
}
table.calmonth th, table.calmonth td {
    width: 12%;
    border: thin black solid;
}
table.calmonth th {
    text-align: center;
}
table.calmonth td.blank {
    border-width: 0;
}
table.calmonth div.daynum {
    float: right;
    width: 1.5em;
    text-align: center;
    color: #03C;
    background-color: lightblue;
    font-size: larger;
    border: thin solid black;
}
table.calmonth td > ul {
    clear: right;
}
<table class="calmonth">
<caption>February</caption>
<tr><th>S</th><th>M</th><th>T</th><th>W</th><th>T</th><th>F</th><th>S</th></tr>
<tr>
<td class="blank"></td><td><div class="daynum">1</div><ul></ul></td><td><div class="daynum">2</div><ul></ul></td><td><div class="daynum">3</div><ul></ul></td><td><div class="daynum">4</div><ul></ul></td><td><div class="daynum">5</div><ul></ul></td><td><div class="daynum">6</div><ul></ul></td></tr>
<tr>
<td><div class="daynum">7</div><ul></ul></td><td><div class="daynum">8</div><ul></ul></td><td><div class="daynum">9</div><ul></ul></td><td><div class="daynum">10</div><ul></ul></td><td><div class="daynum">11</div><ul></ul></td><td><div class="daynum">12</div><ul></ul></td><td><div class="daynum">13</div><ul></ul></td></tr>
<tr>
<td><div class="daynum">14</div><ul></ul></td><td><div class="daynum">15</div><ul></ul></td><td><div class="daynum">16</div><ul></ul></td><td><div class="daynum">17</div><ul></ul></td><td><div class="daynum">18</div><ul></ul></td><td><div class="daynum">19</div><ul></ul></td><td><div class="daynum">20</div><ul></ul></td></tr>
<tr>
<td><div class="daynum">21</div><ul></ul></td><td><div class="daynum">22</div><ul></ul></td><td><div class="daynum">23</div><ul></ul></td><td><div class="daynum">24</div><ul></ul></td><td><div class="daynum">25</div><ul></ul></td><td><div class="daynum">26</div><ul></ul></td><td><div class="daynum">27</div><ul></ul></td></tr>
<tr>
<td><div class="daynum">28</div><ul></ul></td><td><div class="daynum">29</div><ul></ul></td></tr>
</table>

Code quality

Your createCalendar() method is hard-coded to send its output to a certain file. For flexibility, I would split it up into two: one method responsible for generating the HTML, and another for writing it to the appropriate file. Someday you might want to have the output directly written out on-the-fly from your Servlet engine to the client. Or, you could write a unit test that doesn't touch the filesystem.

I suggest renaming the method to createHtmlMonthCalendar to be precise.

If you don't know what to do with an exception, declare the exception and let it propagate. Here, your jButton1ActionPerformed() should catch it and display an alert box.

Repeated string concatenation is highly discouraged for performance scalability. Usually, a StringBuilder would be better. In this case, since you intend to write the output out, I recommend a Writer.

The //loop through rest of first week is partly to blame for the extra <tr> bug. In any case, copy-and-pasted code is undesirable. I suggest that instead of counting from 1 to the end of the month, you iterate using a GregorianCalendar object instead. Advantages are:

  • You can switch the output based on whether the current date is in the right month.
  • You can query the object for the day of the week, which makes the code more expressive.
  • You can pass the date as a callback to fetch the list of events for that day.
  • It's robust in odd situations like the Julian-to-Gregorian transition.

To obtain the filename, you can take full advantage of SimpleDateFormat instead of performing string concatenation. For cross-platform compatibility, use System.getProperty("java.io.tmpdir") instead of hard-coding "C:/temp/".

Avoid declaring FileWriter far away from its point of use, and avoid setting it to null for no good reason. Also, use a try-with-resources block so that you don't need to close() it manually.

Suggested solution

public void createHtmlMonthCalendar(GregorianCalendar month)
        throws IOException {
    Format pathFmt = new SimpleDateFormat("MMMYYYY'.html'");
    File path = new File(System.getProperty("java.io.tmpdir"),
                         pathFmt.format(month.getTime()));
    try (FileWriter fw = new FileWriter(path)) {
        createHtmlMonthCalendar(month, fw);
    }
}

public void createHtmlMonthCalendar(GregorianCalendar month, Writer out)
        throws IOException {
    month = (GregorianCalendar)month.clone();
    month.set(Calendar.DATE, 1);

    GregorianCalendar nextMonth = (GregorianCalendar)month.clone();
    nextMonth.add(Calendar.MONTH, 1);

    Format titleFmt = new SimpleDateFormat("MMMM");
    out.write("<table class=\"calmonth\">\n<caption>");
    out.write(titleFmt.format(month.getTime())); // should be HTML-escaped
    out.write("</caption>\n");

    // Rewind to the start of the week, then rewind another week as
    // a hack to get the headings
    GregorianCalendar d = (GregorianCalendar)month.clone();
    d.add(Calendar.DATE,
          month.getFirstDayOfWeek() - month.get(Calendar.DAY_OF_WEEK) - 7);

    // week names header
    SimpleDateFormat weekAbbrev = new SimpleDateFormat("E");
    out.write("<tr>");
    for (int i = 0; i < 7; i++, d.add(Calendar.DATE, 1)) {
        out.write("<th>");
        out.write(weekAbbrev.format(d.getTime()).charAt(0));
        out.write("</th>");
    }
    // hack: the TR for the header will be immediately closed below
    assert d.get(Calendar.DAY_OF_WEEK) == month.getFirstDayOfWeek();

    // body
    SimpleDateFormat dayNum = new SimpleDateFormat("d");
    for (; d.before(nextMonth); d.add(Calendar.DATE, 1)) {
        if (d.get(Calendar.DAY_OF_WEEK) == month.getFirstDayOfWeek()) {
            out.write("</tr>\n<tr>\n");
        }
        if (d.get(Calendar.MONTH) != month.get(Calendar.MONTH)) {
            out.write("<td class=\"blank\"></td>");
        } else {
            out.write("<td><div class=\"daynum\">");
            out.write(dayNum.format(d.getTime()));
            out.write("</div>");

            String events = getDayContents((GregorianCalendar)d.clone());
            if (events != null) {
                out.write(events);
            }

            out.write("</td>");
        }
    }
    out.write("</tr>\n</table>\n");
    out.flush();
}

private String getDayContents(GregorianCalendar day) {
    // stub, to be filled in
    return "<ul></ul>";
}

Note that I have made a half-hearted attempt at internationalization. When executed in a European locale, this code can generate calendars that start with Monday. (On the other hand, some details are still wrong. For example, in a Chinese locale, the days of the week should be 日、一、二、三、四、etc., but this code outputs 星、星、星、星、星、etc.)

\$\endgroup\$
  • 1
    \$\begingroup\$ I caught the extra <tr> tag in my live code and fixed it, but didn't update the code on here. That comment aside, though, I thank you for your answer. I've never used StringBuilder or Writer but will in the future. \$\endgroup\$ – Bob Stout Feb 11 '16 at 22:30

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.