Category Archives: Best practices

Enum – comparison of Java and .NET

A useful feature added in Java 1.5 (also known as J2SE 5.0, 2004) is the enum. In .NET enums have been present since the very first version (2002, and as a beta since 2000) but the engineers at Sun managed to learn something from the shortcomings of the enums in .NET and provided more flexibility.

Let’s start with the simplest, the .NET implementation. In .NET all data types, including value types (equivalent of the primitive types) are part of the type hierarchy, being, indirectly inherited from System.Object (equiv. of java.lang.Object). The enums are just a specialization on top of exact numeric types, by default int (System.Int32). A typical declaration :

public enum Month
{
    January,
    February,
    March,
    April,
    May,
    June,
    July,
    August,
    September,
    October,
    November,
    December,
}

Notice that the compiler is forgiving and doesn’t complain that after the last element we forgot to not place a comma. It will also work, of course, if we don’t place a comma after the last element. Behind the scenes the compiler will generate a value-type inheriting from System.Enum that will have 12 constants. By default these constants we’ll be of type Int32 and their value, again, by default, will start from 0 and increase by 1 for each member. January will be 0 and December will be 11. Casts between the backing type (Int32 in this case) and the Months type will be allowed both at design time and at runtime.

You can also force individual values for each member

public enum Month
{
    January = 3,
    February = 33,
    March = 222,
    April = 14,
    May = 20,
    June = 23,
    July,
    August,
    September,
    October,
    November,
    December,
}

In this case January will be equal to 3, February 33, …, June 23, July 24 (not specified but after a value-specified member, the next member will be the last value + 1 if specific value is not present. You can even force things into a bad situation like so :

public enum Months
{
    January = 1,
    February,
    March,
    April,
    May,
    June,
    July = 1,
    August,
    September,
    October,
    November,
    December,
}

Guess what, not only this is completely valid, but there won’t be just two duplicate values (January and July being equal to themselves, and equal to 1) but also February will be 2, just like August and so on. Of course, this is not recommended. The compiler and the runtime will happily apply your stupid scheme but the humans will be confused. This excess of freedom is not to my liking but I can’t do much about it except counter-recommend it. Ideally you should not have to specify values for typical enums. Except for…
Read more »

Prefix cast or as-cast?

I read today a nice article, from Kathleen Dollard, called To “as” or not to “as”. This is a pain-point for me on which I stumble often, so I decided to write this little rant.

I particularly liked a paragraph from the above-cited article :

One of the things that makes hard bugs hard is when there is a disconnect in time or space between the cause and the symptom. Time is time, space is lines of code, assembly placement, etc. Code can be written to minimize these disconnects. One of the ways to do that is to fail quickly. When application state becomes incorrect, yell about it immediately and rarely continue with the application in an invalid state. A null-reference exception at an unexpected time just makes code more difficult to debug.

I couldn’t express this as good as Kathleen did. Make no mistake I am quite biased in this comparison (direct-cast vs. as-cast). I kind of hate the abuse of the as operator.

Very often people turn to as instead of the direct (prefix) cast because:

  • They fear the InvalidCastException (strange, they don’t seem to fear the NullReferenceException)
  • They feel the syntax more fluent, closer to the human language.

I would consider the only valid case to use the as-cast is, just like Kathleen states, when a null value result is valid for the rest of the execution of the code. For the rest of the cases it’s just wrong.

This also promotes (doesn’t necessarily causes but promotes) bad practices like this :


public static void OnButtonClick(object sender, EventArgs e)
{
    var button = sender as Button;
    if (button == null)
    {
        return;
    }
    if (button.Tag == "somevalue")
    {
        // do something
    }
    // ...
}

In this example the event handler (which could be attached to more than one distinct button) simply forces under the rug a situation which would be abnormal (the sender not being a button) instead of releasing it so the developers could find it easier and debug it. A saner approach is :


public static void OnButtonClick(object sender, EventArgs e)
{
    var button = (Button)sender;
    if (button.Tag == "somevalue")
    {
        // do something
    }
    // ...
}

This brings me to another advantage of the prefix-cast : it produces shorter, clearer code.

In other cases the as abuse does more harm, hiding the source of a bug :


public void ProcessData(Entity entity)
{
    var person = entity as Person;
    UpdatePersonStatistics(person);
    // .. more code
}

public void UpdatePersonStatistics(Person person)
{
    NormalizeData(person);
    // .. more code
}

public void NormalizeData(Person person)
{
    person.Name = person.Name.Substring(0, 50);
    person.Address = person.Address.Substring(0, 100);
    // .. more code
}

Of course this is a contrived example full of bad practices but for now let’s focus on the as usage. Suppose the ProcessData method receives an instance of Category by mistake. Since Category inherits Entity the compiler will not complain.

The result is that there will be a NullReferenceException two methods further, in the NormalizeData method. If the cast was done with a prefix cast the error was a little bit easier to spot. This is confusing two-fold :

  1. The name of the exception suggests that a null reference was somehow obtained but in fact a real instance of Category was passed, not a null
  2. The error does not originate from the NormalizeData code but from the caller of the ProcessData

Summary

Use as only if a null result of the conversion makes sense for the flow of the execution. Otherwise use prefix cast.

Beware of switch

I am by no means against any of the instructions which exist in C#, including goto (wow, I said that). Each of them can have its place and its normal use in an application. However things can be abused and code gets nasty.

For example let’s consider we are displaying a hierarchy of people in an organization in a tree-like visual component. We will create a class that models a person :

public class Person
{
    public string Name { get; set; }
    public IEnumerable Subordinates { get; set; }
}

All is nice but some requests come in, such as : each type of person should have a distinctive icon at its left, the managers should have salutation type displayed etc.

Then you think “I know, I’ll create a PersonType enum”. That’s when things will begin to get nasty.

public enum PersonType
{
    Undefined,
    Simple,
    Manager
}

public class Person
{
    public IEnumerable Subordinates { get; set; }

    // new members :
    public PersonType Type { get; set; }
    public string Salutation { get; set; }
    private string _name;

    // modified members :
    public string Name
    {
        get
        {
             switch(Type)
             {
                 case PersonType.Simple: return _name;
                 case PersonType.Manager : return Salutation + " " + _name;
                 default : throw new InvalidOperationException();
             }
        }
        set { _name = value; }
    }

    public Image Icon
    {
        get
        {
            switch(Type)
            {
                 case PersonType.Simple: return Icons.SimplePersonIcon;
                 case PersonType.Manager : return Icons.ManagerIcon;
                 default : throw new InvalidOperationException();
            }
        }
    }
}

If you find yourself writing switches in one or more properties or methods of the class, switching on the same thing (PersonType here) then stop it. You are writing hard-to-maintain and error-prone code. Plus its inelegant. This practically is a cry for polymorphism.

The right thing to do™ is to have three classes : an abstract Person class, a SimplePerson class (inheriting the Person class) and a ManagerPerson class (also inheriting the Person class) :


public abstract class Person
{
    public virtual string Name { get; set; }
    public IEnumerable Subordinates { get; set; }
    public abstract Image Icon { get; }
    public string Salutation { get; set; }
}

public class SimplePerson : Person
{
    public override Image Icon { get { Icons.SimplePersonIcon; } }
}

public class ManagerPerson : Person
{
    private string _name;

    public override Image Icon { get { Icons.ManagerIcon; } }
    public override string Name
    {
        get
        {
            return Salutation + " " + _name;
        }
        set
        {
            _name = value;
        }
    }
}

Notice that the code is shorter, simpler to follow and understand. Furthermore although we did introduce two more classes, we also removed the PersonType enum so at a total we introduced only one more class.

WP Like Button Plugin by Free WordPress Templates