11 May 2014

why one shouldn't ignore code compile warnings

Recently I was working on a code re factoring; A method in class had duplicated code and wasn't following the DRY principle.

The method under question was building 3 similar objects but of different types. The code looked like below in simple:

public class Car 
{
public string Type{get;set;}
}

public List<Car> BuildCars()
{
  List<Car> cars = new List<Car>();

  // Build car of type 1
   Car type1Car = new Car(){Type=1};
   cars.Add(type1Car);

  // Build car of type 2
   Car type2Car = new Car(){Type=2};
   cars.Add(type2Car );

  // Build car of type 3
   Car type3Car = new Car(){Type=3};
   cars.Add(type3Car);

   return cars;
}

As one can see a lot of code is duplicated in the method BuildCars(). Duplicate code will suffer from below syndromes:

  • Duplicated code will not have good readability as the methods looks lengthy.
  • And also the code isn't maintainable as it is difficult to debug and apply defect fixes.

Hence I started with re factoring of the method. Created a new method BuildCar() like below:

public Car BuildCar(string type)
{
  Car car = new Car() {Type=1};
  return car;
}

Later as part of the refactoring added a new BuildCar() method and used it in the BuildCars(). Hence code become little more readable and more maintainable because of reduction in duplicate code.

public List<Car> BuildCars()
{
   List<Car> cars = new List<Car>();

   Car car= BuildCar(1);
   cars.Add(car);

   car= BuildCar(2);
   cars.Add(car);

   Car car= BuildCar(3);
   cars.Add(car);

   return cars;
}

Started debugging the code and found that cars returned from BuildCars() method are all of type 1!. Something went wrong with refactoring then. It didn't take much time to find the issue and correct it. Although it wasted some time!.

Corrected the BuildCar() method to use the input parameter type and everything started working as expected. The input variable 'type' was never used and 'type' was hardcoded to 1.

public Car BuildCar(string type)
{
  Car car = new Car() {Type=type};
  return car;
}

Then how one can prevent these kinds mistake from happening again?

Visual Studio will give a warning if a variable isn't used by default. So if I had used the Build Warnings window to see the "unused variable" warning then the mistake would have been prevented and there wouldn't have been a defect in my code.

So moral of the story is ALWAYS ENSURE ZERO WARNINGS! in your code.

No comments:

Post a Comment