Friday, January 30, 2009

Efficient Code

Writing code may look easy to some and hard to others, but some programmers always tend to write messy, smelling, ugly-looking, inefficient code. Here’s one :

protected override AppointmentForm CreateAppointmentForm(SchedulerControl control, Appointment apt, bool openRecurrenceForm)
{
//return new CustomAppointmentForm(control, apt, openRecurrenceForm);
if (Thread.CurrentThread.CurrentUICulture.Name == "fa-IR")
{
if (openRecurrenceForm)
return new CustomAppointmentForm(control, apt,openRecurrenceForm);
else
return new
CustomAppointmentForm(control, apt);
}

if (openRecurrenceForm)
return new AppointmentForm(control, apt,openRecurrenceForm);
else
return new
AppointmentForm(control, apt);
}
What’s wrong with it you might say? Other than the logic implemented wrong (for specific culture insteam of all RTL cultures), “A lot”, I’d answer.

  • Commenting out code instead of deleting it will leave a lot of ‘Ghost’ codes in your source. If you have a source-control in place why do you need to keep old code as commented out? it is always available in your source control, isn’t it??

  • Embedding culture-specific code in not a good idea. What we want to achieve here can be easily achieved using factory patterns.

  • Control flow is very important for other programmers to understand what you wanted to achieve. Writing nested if / else and multiple returns in a method tend to make this harder.

  • Having braces in C style programmings is more a matter of style, but always be consistent.

  • Efficiently use constructor / method overloads. Know what are the default values for overloaded parameters.

The above code could be refactored into this :

protected override AppointmentForm CreateAppointmentForm(SchedulerControl control, Appointment apt, bool openRecurrenceForm)
{
AppointmentForm form = null;

if (CultureHelper.IsCultureRightToLeft)
{
form = new CustomAppointmentForm(control, apt, openRecurrenceForm);
}
else
{
form = new AppointmentForm(control, apt, openRecurrenceForm);
}

return form;
}

Submit this story to DotNetKicks Shout it

No comments: