Create generic HTML calendar

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 = " \n"
+ " \n"
+ "  S\n"
+ "  M\n"
+ "  T\n"
+ "  W\n"
+ "  T\n"
+ "  F\n"
+ "  S\n"
+ " \n"
+ " \n";

//add leading "blank" days for Sun-Sat format and count
for (int x = 1; x   \n";
}
//loop through rest of first week
int daysRemainingInWeek = 7 - startDayOfWeek;
for (int y = 0; y \

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] )

$dayCounter





#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.

StackExchange Code Review Q#119669, answer score: 4