Exempel på dålig kod

Det var ett tag sedan jag skrev och nu några dagar innan Jönköping Developer Dojo har sin stora träff med Dag König från Microsoft kanske jag ska plocka upp bollen igen.

Den här gången tänkte jag ge några exempel på dålig kod. Exemplen ligger publikt på internet och går säkert att leta upp om man vet hur man ska använda en sökmotor.

Exempel 1

 if ((campaignTypeInfoCarrier.Key != CampaignVoucherSpecialPrice.EXCEED_AMOUNT) &&
  (campaignTypeInfoCarrier.Key != CampaignVoucherSpecialPrice.AMOUNT_DISCOUNT) &&
  (campaignTypeInfoCarrier.Key != CampaignVoucherSpecialPrice.PERCENTAGE_DISCOUNT) &&
  (campaignTypeInfoCarrier.Key != CampaignVoucherSpecialPrice.NO_OF_VOUCHER_CODES) &&
  (campaignTypeInfoCarrier.Key != CampaignVoucherSpecialPrice.PRICE_LIST_NAME) &&
  (campaignTypeInfoCarrier.Key != BaseCampaign.CURRENCY_CODE))
     {
         campaignTypeInfoCarrier.CarrierState.IsMarkedForDeleting = true;
     }
 }

Så vad är fel med den kodsnutten? Jo, den är ju riktigt osmidig att läsa vad den syftar på. Bättre hade varit att bryta ut vilkoret i if-satsen i en metod som kanske skulle kunna heta isCampainTypeReadyForDeletion. Då behöver jag inte lusläsa koden för att förstå vad som ska hända. Som en generell rekomendation skulle jag säga att man mycket noga ska välja de tillfällen som man har && eller || i en if-sats. Det gör livet mycket svårare för läsaren.

Exempel 2

 foreach (CampaignTypeInfoCarrier campaignTypeInfo in campaign.CampaignTypeInfos)
{
 if (campaignTypeInfo.Key == CampaignVoucherSpecialPrice.EXCEED_AMOUNT)
 {
     c_textBoxExceedAmount.Text = campaignTypeInfo.Value;
 }
 else if (campaignTypeInfo.Key == CampaignVoucherSpecialPrice.AMOUNT_DISCOUNT)
 {
     c_textBoxAmountDiscount.Text = campaignTypeInfo.Value;
     c_textBoxAmountDiscount.Enabled = true;
     c_radioButtonAmountDiscount.Checked = true;
     c_requiredFieldValidatorAmountDiscount.Enabled = true;
     c_textBoxPercentageDiscount.Text = string.Empty;
     c_textBoxPercentageDiscount.Enabled = false;
     c_radioButtonPercentageDiscount.Checked = false;
     c_requiredFieldValidatorPercentageDiscount.Enabled = false;
 }
 else if (campaignTypeInfo.Key == CampaignVoucherSpecialPrice.PERCENTAGE_DISCOUNT)
 {
     c_textBoxPercentageDiscount.Text = campaignTypeInfo.Value;
     c_textBoxPercentageDiscount.Enabled = true;
     c_radioButtonPercentageDiscount.Checked = true;
     c_requiredFieldValidatorPercentageDiscount.Enabled = true;
     c_textBoxAmountDiscount.Text = string.Empty;
     c_textBoxAmountDiscount.Enabled = false;
     c_radioButtonAmountDiscount.Checked = false;
     c_requiredFieldValidatorAmountDiscount.Enabled = false;
 }
 else if (campaignTypeInfo.Key == CampaignVoucherSpecialPrice.NO_OF_VOUCHER_CODES)
 {
     c_textBoxVoucherCode.Text = campaignTypeInfo.Value;
 }
 else if (campaignTypeInfo.Key == CampaignVoucherSpecialPrice.PRICE_LIST_NAME)
 {
   c_textBoxPriceListName.Text = campaignTypeInfo.Value;
 }
}

Så vad är fel här då? Här är det de långa if-satsen som är problemet. Här kollar man ett och samma data flera gånger och utifrån vad som står agerar man på olika sätt. Bättre hade varit om man hade haft en klass för varje fall och anropat en metod i den klassen som hade gjort rätt jobb enbart utifrån det data som klassen själv äger. Slutresultatet skulle kanske kunna se ut så här:

 foreach (CampaignTypeInfoCarrier campaignTypeInfo in campaign.CampaignTypeInfos) { 
     campaignTypeInfo.setParameters(this); 
} 

Naturligtvis har man gjort en del jobb för att nå det här resultatet men jag skulle nog säga att det är väl spenderad tid.

Det var allt för denna gång.

Första träff med Jönköping Developer Dojo

Då var det bestämt. Jönköping Developer Dojo ska ha sin första träff torsdagen den 18:e mars.

Jönköping Developer Dojo är tänkt att bli en träffpunkt för programmerare i Jönköping med omnejd där vi ska kunna lära av varandra, utbyta erfarenheter och kanske till och med ha lite kul samtidigt.

Om du vill vara med så registrerar du dig på LinkedIn (om du inte redan har ett konto) och söker sedan upp gruppen "Jönköping Developer Dojo" och går med i den.