Sep 18 2014

Tackling the Arrow head Anti-Pattern

Category: .Net | C# | Design Principles ashish sheth @ 00:13

Frequently you will see following type of code in your code base.

if(result != 1)
{
    if(someOtherResult == 101)
    {
        if(anotherValue == 500)
        {
            // do something
        }
    }
    else
    {
        // do some other thing
    }
}

return;

Here the code forms a shape of an arrow-head, as below:

if
    if
        if
            if
                do something
            end
        end
    end
end

If you see, the main logic is deep down into the nested condition, it increases the cyclomatic complexity of the code.

A better version of the same code could be as below:

if(result == 1)
{
    return;
}

if(someOtherResult == 101 && anotherValue == 500)
{
    // do something
    return;
}

// do some other thing

return;

The above code does a number of things to flatten the code and make it better:

  1. Validations are performed first and it returns at the first opportunity.
  2. Multiple nested conditions are combine into one (with “&&”(logical And) operator. If there are multiple expressions forming one such condition, they can be moved to a separate method returning boolean. That method can then be use in the if condition as below:
if(IsValidResult(someOtherResult, anotherResult)
{
    // do something
    return;
}

bool IsValidResult(int someOtherResult, int anotherResult)
{
    if(someOtherResult == 101 && anotherValue == 500)
    {
        return true;
    }
    return false;
}

Tags: , ,

Sep 16 2010

Use Properties to Encapsulate the Hidden Fields or ViewState in Asp.Net

Category: .Net | asp.net | C# | VS2008 ashish sheth @ 05:16

In asp.net you might be using lot of hidden variables and ViewState to maintain state between page postbacks. This can make your code look cluttered. You can use properties to encapsulate the hidden fields or ViewState.

If you are writing lot of code like this:

if(myHidden.Value == string.Empty)
{
	myHidden.Value = "someValue"; 	
}

someVariable = myHidden.Value

You can use properties to encapsulate the access to myHidden field. For example:

public string MyHiddenFieldValue
{
	get
	{
		if(myHidden.Value == string.Empty)
		{
			myHidden.Value = "someValue"; 	
		}
		return myHidden.Value
	}
	set
	{
		myHidden.Value = value;
	}
}

Then you can access the myHidden field just by the property

someVariable = MyHiddenFieldValue;

Similarly if you are storing some custom values in the ViewState of the page the you can create property for the ViewState also.

public string MyCustomViewState
{
	get
	{
		if(ViewState["MyViewState"] == null)
		{
			ViewState["MyViewState"] = "someValue"; 	
		}
		return ViewState["MyViewState"].ToString();
	}
	set
	{
		ViewState["MyViewState"] = value;
	}
}

Tags: , , ,

Apr 24 2010

Violating Single Resposibility Principle using VisualStudio Region

Single Responsibility Principle (SRP)says that "THERE SHOULD NEVER BE MORE THAN ONE REASON FOR A CLASS TO CHANGE.". Although this article mentions only about class, I think the SRP also applies to methods within the class: there should never be more than one reason for a method to change.
Visual Studio provides a good way to mark off section of file in the form of "region"so they can be collapsible and the code can be organized. Many people use region in a big method to organize the code. For example:

public void CreeateOrder(/*some parameters*/)
{
	#region Validate the parameters
	//code goes here
	#endregion

	#region create the order
	//insert the order data in the database
	#endregion

	#region create the order item
	//insert the item data in the database
	#endregion
}

Note that not all people use regions like this. Many people use comments instead of regions in this kind of methods.
As you can see this is a clear violation of the single responsibility principle. The method does more than one thing: it validates the order data, create a top level order and create order items. This can certainly be put into separate method.

private bool ValidateOrderData(/*some parameters*/)
{
	#region Validate the parameters
	//code goes here
	#endregion
}

private bool InsertOrder(/*order related parameter*/)
{
	#region create the order
	//insert the order data in the database
	#endregion
}

private bool InsertOrderItem(/*order item related parameter*/)
{
	#region create the order item
	//insert the item data in the database
	#endregion
}

public  void CreateOrder(/*Some parameter*/)
{
	If(ValidateOrder(/*parameter list*/))
	{
		if(InsertOrder(/*order parameter*/))
		{
			InsertOrderItem(/*order item parameter*/);
		}	
	}
}

As you can see, wherever you are using "region" in your method to demarcate the code, you can very well put that code in a separate method.

Tags: , , , ,

Aug 8 2009

Changing the default name of indexer

Category: .Net | .Net Framework | C# ashish sheth @ 12:05

.NET framework contains many hidden gems, which we rarely need and use, in the Base class libraries.

You probably know that when you write an indexer in C#, it will get compiled into two methods called get_Item and set_Item.

So when you declare an indexer like this:

public string this[int number]

it actually compiles into this:

public string get_Item(int number)

Now, did you know that you can have any other name for your indexer if you don't want the name of your indexer to be get_Item? Yes, you can use System.Runtime.CompilerServices.IndexerNameAttribute to change the name into which your indexer will be compiled. Here is the example.

[System.Runtime.CompnailerServices.IndexerName("BinaryFormatOf")]
    public string this[int number]

The above lines will generate a method like below:

public string get_BinaryFormatOf(int number)

I am not sure in what scenario this can be useful but it will certainely usefull if you have indexers which does fit into the getItem-setItem pattern.

Tags: , , ,