Wednesday, July 6, 2011

Never give a chance to make something wrong

Recectly, I've penetrated the truth that if you don't want others do something (bad), never ever give them a chance to do that. It's true in both my real life and in programming. Since my blog is just about software programming, I would like to write about some funny mistakes that people often make. Well, I did make some of them in the past :D

1/ You may forget about the ability of the constructor.

Let's look at following code:
public class UserViewModel
{    
    // These properties must not be null
    public string FirstName { get; set;}
    public string LastName { get; set;}
    public string Email { get; set;}    
}
We can see from the comment in the class above, the author want 3 propreties must have value. It oould be his intention but someone else may need a instance of this class, so he instantiates a UserViewModel object without setting the values for any of those 3 properties. And that could cause problems at runtime since it's not what the author of UserViewModel want. So instead of doing this, we can make the language ensuring that requirement for us:
using System.Diagnostics.Contracts;
public class UserViewModel
{    
    public UserViewModel(string firstName, string lastName, string email)
    {
        Contract.Assert(!string.IsNullOrEmpty(firstName), "firstName must have value");
        Contract.Assert(!string.IsNullOrEmpty(lastName) , "lastName must have value");
        Contract.Assert(!string.IsNullOrEmpty(email)    , "email must have value");
    
        FirstName = firstName;
        LastName = lastName;
        Email = email;
    }
    
    // These properties must not be null
    public string FirstName { get; private set;}
    public string LastName { get; private set;}
    public string Email { get; private set;}    
}

2/ You won't need a private field for a public property.

My team is applying BDD and using PageObject pattern. A guy in a team created a class that have a something like:
private RegistrationStatus _registrationStatus;

public RegistrationStatus Status
{
    get { return _registrationStatus; }
}
What an unnecessary 4 lines of code. You know what, just a few weeks later, the solution is full of these mistakes because alot of people are writing BDD tests and they like copy & paste. So, just change it to 1 line of code version:
 
public RegistrationStatus Status {get; private set;}
 
Well, if you don't want to see these stuff in the code, don't make any thing like this because we're all doing the "COPY & PASTE" job.

3/ If you have not finished something, use NotImplementedException

Apparently working in a team, someone could create a service interface with some methods then the other could use the service for presentation layer. Here is an example:
public interface IUserService
{
    User Get(Guid id);
    int Count();
}

public UserService : IUserService
{
    public User Get(Guid id)
    {
        return null;
    }
    
    public int Count()
    {
        return -1;
    }
}
Everything is fine when compile but why return something like those when they're useless value. Well, if we need an implementation of IUserService, why not create a Mock or a fake object then everyone knows it's a fake and it will never been used in the production. However, it could be acceptable if we are creating a NullObject, but I mostly throw NotImplementedException instead of doing nothing for a void method or return meaningless value for functions.

4/ it's not really a singleton

Everyone knows about singleton. It supposes to be the easiest most simple design pattern. Let's see this code:
public class Helper
{
    private static Helper _instance;
    public static Helper Instance
    {
        get
        {
            if (_instance == null)
            {
                _instance = new Helper();
            }
            return _instance;            
        }
    }
}
Okey, it's just a very simple implementation. But the idea is that it allows only 1 instance of Helper in the system and whenever you want that instance, you must access it from Helper.Instance. Cool but it's not really singleton. Anyone can create an object of Helper easily because by default, the constructor is public so make it private if you don't want people use it.

5/ We should follow Dependency Inversion Principle

This principle is the one that affects my way of thinking. If you read about the Onion architecture, you'll see that architecture is not complicated. It just applies the DIP and make a flashy name. If we apply DIP strickly, we'll never put implementation and interface together in a class library. Instead, put them in separated project. Classes depend on the interface will never know about the concreate implementation of that interface and they will be connected by some sort of IOC library. However, doing that way can make you create alot of projects in one solution. Honestly I have never done anything like this but, but I think Onion architecture is something to consider when you design new application. I prefer isolating the business implementation from the technology we use so any changes of technology will make a little affect to our code.

0 comments:

Post a Comment