Wednesday, March 31, 2010

Improving The Code I Write

Most of the reason I write articles are so that I have a place to put all the cool things i find regarding coding in one place.  Something has recently been coming up that has captured my attention: “Code Smells”.  By Code smells I mean the ability to look or evaluate any piece of code that has been written and assess its value based on a number of indicators. 
The question that started me on this path is: “How many developers look at ways of improving the way they write code?”.  I have been writing recently to try and learn and refresh my knowledge on technologies as way to improve the code I write.  BUT this brings up a big question, does knowing a lot of technologies make you a better coder?  I would assume that the more you know makes you more valuable since the company you work for would not need to train you, but does this make you a better coder?  I don’t think so because you could still write messy code that is difficult to maintain. 
So in my quest to better the code I write, I have been looking at ways of improving and making sure that code I am writing does not have the following smells:

Duplicate Code

One of the best indicators for me is code duplication.  I am sure that people add duplicated code into an application for many reasons: not enough time to extract the logic into a common method library, ease of copy and paste etc.  The reason I put this at the top is that it when you come to maintain an application that has a lot of code duplication or duplicated logic it show on the amount of time it takes to do simple changes.  Many times changes that have been made to a system will not have been updated in all the duplicated methods a lead to easily avoidable bugs being introduced.  The Principle DRY: Don’t Repeat Yourself is a good way of making sure when you are coding to try and look at the bigger picture.  It may take a little longer when you are initially writing the code, but I promise it will save time in the long run.

The God Object

This is something that can creep into any application.  My guess is that the coder is trying to make it simple and keep related logic in one place or the team is being lazy when adding to existing factories/objects.  The problem with this is that you end up with a method that does EVERYTHING and again becomes a nightmare to maintain.  I have seen coders propose frameworks that would be guilty of this, that down the line cause maintenance teams endless hassles.

Overly Long methods

This is closely tied to the god method I mentioned above.  It refers to methods with too much logic and there are hundreds of lines long.  Obviously when another developer comes along they struggle to understand what the purpose of the method is and how everything works.  A solution to this would be to try and break up the method into components that help make it more manageable.

Large chunks of commented out code

I haven’t seen too many complaints regarding this but it i cant stand it when I see it.  What is the point of leaving code commented out for a considerable time.  I obviously don’t mind code commented out if it is causing a problem or will be reinstated after a short period.  But large chunks of code commented out for large periods of time makes no sense.  Most projects I hope are in some kind of source control.  If that is the case the developer could go back find the code if it is needed again and correctly labelled / branched?

Global Variables

Global variables can be bad for many reasons,  I will relate the issues I have had with them.  The reason they show bad practice is that they can be accessed and changed from any method.  They can be changed anywhere not to mention shadowed and increase the likelihood of bugs.  Please don’t misunderstand me,  I have no problem with properties on web forms.  The issue I am talking about would be variable that is used by multiple methods i.e. a Data Set that get set and reset in different places.

Throwing an exception in a catch block

I have never really understood this practice.  If you need to make sure that you are closing streams or there is code that needs to run you can and a finally clause.  The code I am talking about is:
        try 
        { 
            throw new Exception("Test"); 
        } 
        catch (Exception exception) 
        { 
            throw exception; 
        } 
        finally 
        { 
            //Code that needs to run 
        }

Re-throwing the exception destroys the stack trace information and causes headaches when trying to debug the error.
If you need to log the error you could use the following to re-throw the error without destroying the stack trace:
        try 
        { 
            throw new Exception("Test"); 
        } 
        catch (Exception exception) 
        { 
            Logger.LogError(exception) 
            throw;  //Preserves the stack trace 
        } 
        finally 
        { 
            //Code 
        }

Non Descriptive Variable Names

This has to be my pet hate.  In a few projects I have looked at the naming convention seems to be the use of single char variable names as in int a; string b;.  This isnt minified JavaScript, this was working c# code.  There must be some reason people do  it, maybe they want people to not meddle with their code or make it hard to understand.  Personally I stay away from code that does this simply because making any changes could cause errors.  Not to mention that trying to remember what variable is what or used for what can be difficult.
I was going to add Non descriptive Method names as in two or three letter method names but I think it could all go under the same category since it is really the same thing.  I think it goes without saying that while reading through code that references the method ssd which gets back the variable w and goes on to use it in a method named Adw does really go a long way in making the code easy to understand.
In closing the reason I am bringing up these points is that I want to have pride in the work that I do.  I do see it as a work of art.  Not only are we creating things, but they are useful (hopefully) to people that need to get their job done. 
I am always keen on suggestions and ways of keeping my code clean and practices that aid this, so if you have suggestions please reply to this post.

Thursday, March 25, 2010

Delegates and Events in C# .NET

Content


All of us have been exposed to event driven programming of some sort or the other. C# adds on value to the often mentioned world of event driven programming by adding support through events and delegates. The emphasis of this article would be to identify what exactly happens when you add an event handler to your common UI controls. A simple simulation of what could possibly be going on behind the scenes when the AddOnClick or any similar event is added to the Button class will be explained. This will help you understand better the nature of event handling using multi cast delegates.
A delegate in C# is similar to a function pointer in C or C++. Using a delegate allows the programmer to encapsulate a reference to a method inside a delegate object. The delegate object can then be passed to code which can call the referenced method, without having to know at compile time which method will be invoked.
In most cases, when we call a function, we specify the function to be called directly. If the class MyClass has a function named Process, we'd normally call it like this (SimpleSample.cs):
using System;

namespace Akadia.NoDelegate
{
    public class
 MyClass
    {
        public void
 Process()
        {
            Console.WriteLine("Process() begin");
            Console.WriteLine("Process() end");
        }
    }

    public class Test
    {
        static void Main(string[] args)
        {
           
 MyClass myClass = new MyClass();
            myClass.
Process();
        }
    }
}
That works well in most situations. Sometimes, however, we don't want to call a function directly - we'd like to be able to pass it to somebody else so that they can call it. This is especially useful in an event-driven system such as a graphical user interface, when I want some code to be executed when the user clicks on a button, or when I want to log some information but can't specify how it is logged.
An interesting and useful property of a delegate is that it does not know or care about the class of the object that it references. Any object will do; all that matters is that the method's argument types and return type match the delegate's. This makes delegates perfectly suited for "anonymous" invocation.
The signature of a single cast delegate is shown below:
delegate result-type identifier ([parameters]);
where:
o    result-type: The result type, which matches the return type of the function.
o    identifier: The delegate name.
o    parameters: The Parameters, that the function takes.
Examples:
public delegate void SimpleDelegate ()
This declaration defines a delegate named SimpleDelegate, which will encapsulate any method that takes
no parameters and returns no value.
public delegate int ButtonClickHandler (object obj1, object obj2)
This declaration defines a delegate named ButtonClickHandler, which will encapsulate any method that takes
two objects as parameters and returns an int.
A delegate will allow us to specify what the function we'll be calling looks like without having to specify whichfunction to call. The declaration for a delegate looks just like the declaration for a function, except that in this case, we're declaring the signature of functions that this delegate can reference.
There are three steps in defining and using delegates:
o    Declaration
o    Instantiation
o    Invocation
A very basic example (SimpleDelegate1.cs):
using System;

namespace Akadia.BasicDelegate
{
   
 // Declaration
   
 public delegate void SimpleDelegate();

    class TestDelegate
    {
        public static void
 MyFunc()
        {
            Console.WriteLine("I was called by delegate ...");
        }

        public static void Main()
        {
           
 // Instantiation
           
 SimpleDelegate simpleDelegate = new SimpleDelegate(MyFunc);

           
 // Invocation
           
 simpleDelegate();
        }
    }
}
Compile an test:
# csc SimpleDelegate1.cs
# SimpleDelegate1.exe
I was called by delegate ...
For our next, more advanced example (SimpleDelegate2.cs), declares a delegate that takes a single string parameter and has no return type:
using System;

namespace Akadia.SimpleDelegate
{
   
 // Delegate Specification
    public class MyClass
    {
       
 // Declare a delegate that takes a single string parameter
        // and has no return type.

       
 public delegate void LogHandler(string message);

       
 // The use of the delegate is just like calling a function directly,
        // though we need to add a check to see if the delegate is null
        // (that is, not pointing to a function) before calling the function.

        public void Process(
LogHandler logHandler)
        {
            if (logHandler != null)
            {
                logHandler("Process() begin");
            }

            if (logHandler != null)
            {
                logHandler ("Process() end");
            }
        }
    }

   
 // Test Application to use the defined Delegate
    public class TestApplication
    {
       
 // Static Function: To which is used in the Delegate. To call the Process()
        // function, we need to declare a logging function: Logger() that matches
        // the signature of the delegate.

       
 static void Logger(string s)
        {
            Console.WriteLine(s);
        }

        static void Main(string[] args)
        {
            MyClass myClass = new MyClass();

           
 // Crate an instance of the delegate, pointing to the logging function.
            // This delegate will then be passed to the Process() function.

            MyClass.
LogHandler myLogger = new MyClass.LogHandler(Logger);
            myClass.Process(
myLogger);
        }
    }
}
Compile an test:
# csc SimpleDelegate2.cs
# SimpleDelegate2.exe
Process() begin
Process() end
In the simple example above, the Logger( ) function merely writes the string out. A different function might want to log the information to a file, but to do this, the function needs to know what file to write the information to (SimpleDelegate3.cs)
using System;
using System.IO;

namespace Akadia.SimpleDelegate
{
   
 // Delegate Specification
    public class MyClass
    {
       
 // Declare a delegate that takes a single string parameter
        // and has no return type.

       
 public delegate void LogHandler(string message);

       
 // The use of the delegate is just like calling a function directly,
        // though we need to add a check to see if the delegate is null
        // (that is, not pointing to a function) before calling the function.

        public void Process(
LogHandler logHandler)
        {
            if (logHandler != null)
            {
                logHandler("Process() begin");
            }

            if (logHandler != null)
            {
                logHandler ("Process() end");
            }
        }
    }

   
 // The FileLogger class merely encapsulates the file I/O
   
 public class FileLogger
    {
        FileStream fileStream;
        StreamWriter streamWriter;

       
 // Constructor
        public FileLogger(string filename)
        {
            fileStream = new FileStream(filename, FileMode.Create);
            streamWriter = new StreamWriter(fileStream);
        }

       
 // Member Function which is used in the Delegate
        public void
 Logger(string s)
        {
            streamWriter.WriteLine(s);
        }

        public void Close()
        {
            streamWriter.Close();
            fileStream.Close();
        }
    }

   
 // Main() is modified so that the delegate points to the Logger()
    // function on the fl instance of a FileLogger. When this delegate
    // is invoked from Process(), the member function is called and
    // the string is logged to the appropriate file.

    public class TestApplication
    {
        static void Main(string[] args)
        {
            FileLogger fl = new FileLogger("process.log");

            MyClass myClass = new MyClass();

           
 // Crate an instance of the delegate, pointing to the Logger()
            // function on the fl instance of a FileLogger.

            MyClass.
LogHandler myLogger = new MyClass.LogHandler(fl.Logger);
            myClass.Process(myLogger);
            fl.Close();
        }
    }
}
The cool part here is that we didn't have to change the Process() function; the code to all the delegate is the same regardless of whether it refers to a static or member function.
Compile an test:
# csc SimpleDelegate3.cs
# SimpleDelegate3.exe
# cat process.log

Process() begin
Process() end
Being able to point to member functions is nice, but there are more tricks you can do with delegates. In C#, delegates are multicast, which means that they can point to more than one function at a time (that is, they're based off the System.MulticastDelegate type). A multicast delegate maintains a list of functions that will all be called when the delegate is invoked. We can add back in the logging function from the first example, and call both delegates. Here's what the code looks like:
using System;
using System.IO;

namespace Akadia.SimpleDelegate
{
    // Delegate Specification
   
 public class MyClass
    {
       
 // Declare a delegate that takes a single string parameter
        // and has no return type.

        public delegate void
 LogHandler(string message);

       
 // The use of the delegate is just like calling a function directly,
        // though we need to add a check to see if the delegate is null
        // (that is, not pointing to a function) before calling the function.

       
 public void Process(LogHandler logHandler)
        {
            if (logHandler != null)
            {
                logHandler("Process() begin");
            }

            if (logHandler != null)
            {
                logHandler ("Process() end");
            }
        }
    }

   
 // The FileLogger class merely encapsulates the file I/O
   
 public class FileLogger
    {
        FileStream fileStream;
        StreamWriter streamWriter;

       
 // Constructor
        public FileLogger(string filename)
        {
            fileStream = new FileStream(filename, FileMode.Create);
            streamWriter = new StreamWriter(fileStream);
        }

       
 // Member Function which is used in the Delegate
       
 public void Logger(string s)
        {
            streamWriter.WriteLine(s);
        }


        public void Close()
        {
            streamWriter.Close();
            fileStream.Close();
        }
    }

   
 // Test Application which calls both Delegates
    public class TestApplication
    {
      
   // Static Function which is used in the Delegate
       
 static void Logger(string s)
        {
            Console.WriteLine(s);
        }


        static void Main(string[] args)
        {
            FileLogger fl = new FileLogger("process.log");

            MyClass myClass = new MyClass();

           
 // Crate an instance of the delegates, pointing to the static
            // Logger() function defined in the TestApplication class and
            // then to member function on the fl instance of a FileLogger.

           
 MyClass.LogHandler myLogger = null;
            myLogger
 += new MyClass.LogHandler(Logger);
            myLogger
 += new MyClass.LogHandler(fl.Logger);

            myClass.Process(myLogger);
            fl.Close();
        }
    }
}
Compile an test:
# csc SimpleDelegate4.cs
# SimpleDelegate4.exe
Process() begin
Process() end

# cat process.log

Process() begin
Process() end
The Event model in C# finds its roots in the event programming model that is popular in asynchronous programming. The basic foundation behind this programming model is the idea of "publisher and subscribers." In this model, you havepublishers who will do some logic and publish an "event." Publishers will then send out their event only to subscriberswho have subscribed to receive the specific event.
In C#, any object can publish a set of events to which other applications can subscribe. When the publishing class raises an event, all the subscribed applications are notified. The following figure shows this mechanism.

The following important conventions are used with events:
o    Event Handlers in the .NET Framework return void and take two parameters.
o    The first paramter is the source of the event; that is the publishing object.
o    The second parameter is an object derived from EventArgs.
o    Events are properties of the class publishing the event.
o    The keyword event controls how the event property is accessed by the subscribing classes.
Let's modify our logging example from above to use an event rather than a delegate:
using System;
using System.IO;

namespace Akadia.SimpleEvent
{
   
 /* ========= Publisher of the Event ============== */
    public class MyClass
    {
       
 // Define a delegate named LogHandler, which will encapsulate
        // any method that takes a string as the parameter and returns no value

        public delegate void
 LogHandler(string message);

       
 // Define an Event based on the above Delegate
       
 public event LogHandler Log;
 
 
       
 // Instead of having the Process() function take a delegate
        // as a parameter, we've declared a Log event. Call the Event,
        // using the OnXXXX Method, where XXXX is the name of the Event.

        public void Process()
        {
           
 OnLog("Process() begin");
           
 OnLog("Process() end");
        }

       
 // By Default, create an OnXXXX Method, to call the Event
       
 protected void OnLog(string message)
        {
            if (
Log != null)
            {
               
 Log(message);
            }
        }
    }

  
  // The FileLogger class merely encapsulates the file I/O
    public class FileLogger
    {
        FileStream fileStream;
        StreamWriter streamWriter;

       
 // Constructor
        public FileLogger(string filename)
        {
            fileStream = new FileStream(filename, FileMode.Create);
            streamWriter = new StreamWriter(fileStream);
        }

       
 // Member Function which is used in the Delegate
        public void Logger(string s)
        {
            streamWriter.WriteLine(s);
        }

        public void Close()
        {
            streamWriter.Close();
            fileStream.Close();
        }
    }

    /* ========= Subscriber of the Event ============== */
   
 // It's now easier and cleaner to merely add instances
    // of the delegate to the event, instead of having to
    // manage things ourselves

    public class TestApplication
    {
        static void Logger(string s)
        {
            Console.WriteLine(s);
        }

        static void Main(string[] args)
        {
            FileLogger fl = new FileLogger("process.log");
            MyClass myClass = new MyClass();

            // Subscribe the Functions Logger and fl.Logger
           
 myClass.Log += new MyClass.LogHandler(Logger);
            myClass.Log += new MyClass.LogHandler(fl.Logger);

            // The Event will now be triggered in the Process() Method
            myClass.Process();


            fl.Close();
        }
    }
}
Compile an test:
# csc SimpleEvent.cs
# SimpleEvent.exe
Process() begin
Process() end

# cat process.log

Process() begin
Process() end
Suppose you want to create a Clock class that uses events to notify potential subscribers whenever the local time changes value by one second. Here is the complete, documented example:
using System;
using System.Threading;

namespace SecondChangeEvent
{
  
 /* ======================= Event Publisher =============================== */

  
 // Our subject -- it is this class that other classes
   // will observe. This class publishes one event:
   // SecondChange. The observers subscribe to that event.

   public class Clock
   {
    
  // Private Fields holding the hour, minute and second
      private int _hour;
      private int _minute;
      private int _second;

    
  // The delegate named SecondChangeHandler, which will encapsulate
      // any method that takes a clock object and a TimeInfoEventArgs
      // object as the parameter and returns no value. It's the
      // delegate the subscribers must implement.

     
 public delegate void SecondChangeHandler (
         object clock,
         TimeInfoEventArgs timeInformation
      );


     
 // The event we publish
     
 public event SecondChangeHandler SecondChange;

    
  // The method which fires the Event
     
 protected void OnSecondChange(
         object clock,
         TimeInfoEventArgs timeInformation
      )
      {
         // Check if there are any Subscribers
         if (SecondChange != null)
         {
            // Call the Event
            SecondChange(clock,timeInformation);
         }
      }


    
  // Set the clock running, it will raise an
      // event for each new second

      public void Run()
      {
         for(;;)
         {
           
 // Sleep 1 Second
            Thread.Sleep(1000);

           
 // Get the current time
            System.DateTime dt = System.DateTime.Now;

           
 // If the second has changed
            // notify the subscribers

            if (dt.Second != _second)
            {
              
 // Create the TimeInfoEventArgs object
               // to pass to the subscribers

               TimeInfoEventArgs timeInformation =
                  new TimeInfoEventArgs(
                  dt.Hour,dt.Minute,dt.Second);

              
 // If anyone has subscribed, notify them
              
 OnSecondChange (this,timeInformation);
            }

           
 // update the state
            _second = dt.Second;
            _minute = dt.Minute;
            _hour = dt.Hour;

         }
      }
   }

  
 // The class to hold the information about the event
   // in this case it will hold only information
   // available in the clock class, but could hold
   // additional state information

   public class TimeInfoEventArgs : EventArgs
   {
      public TimeInfoEventArgs(int hour, int minute, int second)
      {
         this.hour = hour;
         this.minute = minute;
         this.second = second;
      }
      public readonly int hour;
      public readonly int minute;
      public readonly int second;
   }

 
  /* ======================= Event Subscribers =============================== */

  
 // An observer. DisplayClock subscribes to the
   // clock's events. The job of DisplayClock is
   // to display the current time

   public class DisplayClock
   {
     
 // Given a clock, subscribe to
      // its SecondChangeHandler event

      public void Subscribe(Clock theClock)
      {
        
 theClock.SecondChange +=
            new Clock.SecondChangeHandler(TimeHasChanged);

      }

    
  // The method that implements the
      // delegated functionality

      public void TimeHasChanged(
         object theClock, TimeInfoEventArgs ti)
      {
         Console.WriteLine("Current Time: {0}:{1}:{2}",
            ti.hour.ToString(),
            ti.minute.ToString(),
            ti.second.ToString());
      }
   }

  
 // A second subscriber whose job is to write to a file
   public class LogClock
   {
      public void Subscribe(Clock theClock)
      {
        
 theClock.SecondChange +=
            new Clock.SecondChangeHandler(WriteLogEntry);

      }

     
 // This method should write to a file
      // we write to the console to see the effect

     
 // this object keeps no state
      public void WriteLogEntry(
         object theClock, TimeInfoEventArgs ti)
      {
         Console.WriteLine("Logging to file: {0}:{1}:{2}",
            ti.hour.ToString(),
            ti.minute.ToString(),
            ti.second.ToString());
      }
   }

  
 /* ======================= Test Application =============================== */

  
 // Test Application which implements the
   // Clock Notifier - Subscriber Sample

   public class Test
   {
      public static void Main()
      {
        
 // Create a new clock
         Clock theClock = new Clock();

        
 // Create the display and tell it to
         // subscribe to the clock just created

         DisplayClock dc = new DisplayClock();
         dc.Subscribe(theClock);

        
 // Create a Log object and tell it
         // to subscribe to the clock

         LogClock lc = new LogClock();
         lc.Subscribe(theClock);

        
 // Get the clock started
         theClock.Run();
      }
   }

}
The Clock class from the last sample could simply print the time rather than raising an event, so why bother with the introduction of using delegates? The advantage of the publisg / subscribe idiom is that any number of classes can be notified when an event is raised. The subscribing classes do not need to know how the Clock works, and the Clock does not need to know what they are going to do in response to the event. Similarly a button can publish an Onclick event, and any number of unrelated objects can subscribe to that event, receiving notification when the button is clicked.
The publisher and the subscribers are decoupled by the delegate. This is highly desirable as it makes for more flexible and robust code. The clock can chnage how it detects time without breaking any of the subscribing classes. The subscribing classes can change how they respond to time changes without breaking the Clock. The two classes spin indepentdently of one another, which makes for code that is easier to maintain.

Popular Posts