I’ve just started a unit at University called “Desktop Application Development” which is supposed to give students “an understanding of the processes and techniques used to develop end-user software that includes modern graphical interface components” and “practical experience in constructing desktop applications within an advanced software development environment”. In this case the framework of choice is WinForms via C# and the IDE is Visual Studio 2010.

It all sounds quite reasonable – and for the most part,  it is. The lectures have been good thus far and I’m interested to see what will be discussed later in the term. However, the coursework is a different matter entirely and while the concept seems solid, I feel very strongly that the implementation just isn’t suitable for the course.

Concept

The practical component of the coursework is to develop an image library application which has a simple file-based database of collections and information about the photographs each record points to. I feel this is a good coursework task because:

• it requires implementation of file IO
• the end application will be fairly complex, making use of multiple windows and dialogs (including custom and common dialogs)
• the final product should be useful in some way
• the final product can be extended to add extra features.

However, with the good out the way, now we get the the weird…

The Weird

Rather than getting students to start from scratch, a project is provided as a starting point, with the main form already populated with the required controls and even some predefined methods. Three other dialog forms are also included (similarly pre-populated) along with complete classes for handling images and image collections.

At first I was really surprised by this (questioning why not encourage students to build an application completely from scratch) but after a little thought I can see the motivation behind this decision. I think they want students to focus on the more important aspects of the application. However, I can’t help feeling that too much has been done already, in particular I think the image and image collection classes shouldn’t just be given to students as a whole, but I’ll let that slide because my main concerns are with how the existing code base was written.

WAT.

Starting with the main form (because that’s the first thing I saw), straight away I see something a little… odd:

You’re supposed to be teaching us about GUI design and you use non-system font in the coursework template? What about core UI design concepts like consistency and design guidelines? Do they mean nothing to you?

Moving on from the non-standard font choice, what else is there?

It looks like the components in the coloured areas are grouped together into relevant containers, which would be nice. Except, those coloured areas aren’t containers at all. They’re not even coloured rectangles. They’re actually coloured labels with an empty text property. Wat. Seriously. They’re not even aligned properly!

Not only is this a strange choice for adding colour to an application, but also a bad one. What if someone using the high-contrast theme wanted to use the application?

Humorously the text is now even less readable now because the theme set the default colour to white (expecting a dark background).

So what about the code? Well other than using Hungarian notation everywhere and prefixing classes with ‘C’ (personal dislike) and enums with ‘e’ (which I have never seen before) the codebase isn’t all that bad. However, there are a number of inefficiencies which I would normally overlook except that this project will set the standard to which students will tend to write their code. For example:

public CPSImage()

{

strFileName = "empty";

lstCategories = new List<eCategory>();

bmpImage = null;

lstKeywords = new List<string>(); //list of keywords associated with the image

strComment = "empty";

rftOrientation = RotateFlipType.RotateNoneFlipNone;

}

public CPSImage(string strFileLoc)

{

strFileName = strFileLoc;

lstCategories = new List<eCategory>();

lstKeywords = new List<string>(); //list of keywords associated with the image

strComment = "empty";

bmpImage = null;

rftOrientation = RotateFlipType.RotateNoneFlipNone;

}

Considering that one of the core mantras taught to students throughout the course is to minimize duplicating functionality in your code, it seems very odd to have it happen all over the place in this project.

Of course, in order to avoid duplication of code, this should really be written as:

public CPSImage(string strFileLoc)

{

strFileName = strFileLoc;

lstCategories = new List<eCategory>();

lstKeywords = new List<string>(); //list of keywords associated with the image

strComment = "empty";

bmpImage = null;

rftOrientation = RotateFlipType.RotateNoneFlipNone;

}

public CPSImage() : this("empty")

{

}

And even then I protest the default definition of any string to “empty” (why not use an empty string?).

Although it has never been suggested at any point that one should improve the template to improve it, I will certainly be doing exactly that. However, if this was actually expected of us, it was never made clear and I highly doubt anyone else will bother.