Introduction
Today I was reading an article on Codeproject, and I was really astounded by the way some people rate articles and give comments...
It was an article about someone who had implemented a simple service locator which contained the following code sample :
using System;
using CuttingEdge.ServiceLocation;
using Microsoft.Practices.ServiceLocation;
public class Global : System.Web.HttpApplication
{
protected void Application_Start(object sender, EventArgs e)
{
// 1. Create a new Simple Service Locator container
var container = new SimpleServiceLocator();
// 2. Configure the container
// Register a delegate that will create a new
// instance on each call to GetInstance.
container.Register<ISamurai(() =>
{
var weapon = ServiceLocator.Current .GetInstance();
return new Samurai(weapon);
});
// Register a single object instance that always
// be returned (must be thread-safe).
container.RegisterSingle(new Katana());
// 3. Register the container to the Common Locator
ServiceLocator.SetLocatorProvider(() => container);
}
}
Which seems quite obvious to me. Since I noticed a comment titled 'my vote of 2', I was curious why this article would be voted such a low score (2 out of 5). I checked the comment and I was astounded:
Please note: the keyword "var" is supposed to be used with anonymous types. You should not use it for anything else. Otherweise please start
programming java script
Who cares ?
Actually, I do.
Since I could not resist I posted the following comment:
Ahum... there have been numerous issues about this, but in general it should not pose a problem, because it is quite clear what it's intentions are at the beginning; i.e. :
var sl = new SimpleServiceLocator();
Is not really more obvious then :
SimpleServiceLocator sl = new SimpleServiceLocator();
In fact, the information in there is the same, but the second one is more verbose, so every person in his right mind would opt for the first alternative.
This might be different if you use a function which does not directly expose the return type in the function name; i.e. :
var x = SomeFunction(y,z);
This is considered bad practice by most.
This one is not :
var myservice = sl.Resolve<IMyService>();
Because the initialization contains enough obvious info to give pointers to the var's type.
So, if this is the only reason why you are giving a 2 (the author has obviously invested quite some time in this article), then you might need to start doing some JavaScript yourself
After tweeting about this, I had an interesting conversation with @JacoPretorious about this, who says he uses var everywhere, but pays more attention to variable names.
While I understand what he means, IMHO variable names should not infer the type, only the meaning of the content if necessary.
I personally think there is nothing wrong with using single letters as variable names, as long as the intent is obvious. This is an example that is perfectly valid imho :
public bool InvalidateCustomer(int CustomerId)
{
var c = sRepo.GetById<Customer>(CustomerId);
if ( c.Orders.Where(o=>o.PaymentReceivedOn==null).Any())
return false;
c.Valid=false;
sRepo.SaveOrUpdate(c);
sUOW.Commit();
return true;
}
This one is not IMO :
public class MyProcessQueue
{
public void ProcessQueue()
{
for(;;)
{
var p = Queue.Next();
if (p==null)
break;
sProcessor.Process(p);
}
}
}
Since you can not infer the type from Queue, you should make the type explicit :
public class MyProcessQueue
{
public void ProcessQueue()
{
for(;;)
{
Order o = Queue.Next();
if (o==null)
break;
sProcessor.Process(o);
}
}
}
This would change if the MyProcessQueue would be a generic class :
public class MyProcessQueue<T>
where T:ISomeInterface
{
public void ProcessQueue()
{
for(;;)
{
var p = Queue.Next();
if (p==null)
break;
sService.Process(p);
}
}
}
In this case, I think you have enough information available to know that the type should be about the generic class. Using more explicit names for the variables does not add a lot info for me; I do not find the following (exagerated) example more abvious than the previous one; in fact, it takes more time to get the intent here, because your mind needs to process more information (also note the comments, which do not provide any extra info to me) :
public class MyProcessQueue<T>
where T:ISomeInterface
{
// Process all jobs in the queue, until there is nothing left
public void ProcessAllISomeInterfacesInTheQueue()
{
// loop forever
for(;;)
{
// get the next instance to process from the queue;
// if it returns null there are no more processes left on the queue
ISomeInterface TheNextISomeInterfaceInstanceToProcess = Queue.Next();
if (TheNextISomeInterfaceInstanceToProcess==null)
{
//exit the infinite loop
break;
}
// process it
sServiceThatProcessesSomeInterfaceInstance
.Process(TheNextISomeInterfaceInstanceToProcess);
// go to the next one
}
// return
}
}
There is way too much info here which can all be easily derived from the code, and if you change the code and forget the comments, this might actually do the opposite: cause confusion.
In conclusion
While this is all about preference and highly dependent on you point of view and background, I do think that the approach I am personnaly using is one of the better ones. They offer all the amount of information that you could need with the minimum amount of code.
If you do have another opinion, do not hesitate to let me know...
Subscribe