1
\$\begingroup\$

Say I have a code snippet like this

    if(string.Compare(isStandardFlag,"STANDARD",true)==0)
    {
        hearingID = centreSession.standardID;
        centreSession.standardOutcomeID 
            = disciplineSvc.SaveResultA(hearingID,OffenceID, Outcome);
        centreSession.standardOutcomeNoteID
            = disciplineSvc.SaveResultB(hearingID, txt1.Text, "ResultHearing"); 
    }
    else{
        hearingID = centreSession.noneStandardID;
        centreSession.noneStandardHearingOutcomeID
            = disciplineSvc.SaveResultA(hearingID,OffenceID, Outcome);
        centreSession.noneStandardHearingOutcomeNoteID
            = disciplineSvc.SaveResultB(hearingID, txt1.Text, , "ResultHearing"); 
    }

You can see the 'if' and 'else' do the same thing but just assign the result to different variables.

Is there a pattern I can refer to refactor the code?

\$\endgroup\$
  • \$\begingroup\$ What is the reason that you need to save the results in different properties? \$\endgroup\$ – Jeff Vanzella Apr 29 '13 at 16:35
  • \$\begingroup\$ @JeffVanzella it is a usercontrol been used in 2 different places on the same screen, the logic is very similar. I need 2 places to store their results. \$\endgroup\$ – ValidfroM Apr 29 '13 at 16:40
3
\$\begingroup\$

First, it looks like the ID, OutcomeID and OutcomeNoteID belong together, so I think it it makes sense to extract them into a separate class (though my naming of it is just a guess):

class Hearing
{
    public int ID { get; private set; }

    public int OutcomeID { get; private set; }

    public int OutcomeNoteID { get; private set; }

    public Hearing(int id)
    {
         ID = id;
    }

    public void SetOutcome(int outcomeId, int outcomeNoteId)
    {
        OutcomeID = outcomeId;
        OutcomeNoteID = outcomeNoteId;
    }
}

With that, your code could be changed to something like:

Hearing hearing;
if (string.Equals(isStandardFlag, "STANDARD", StringComparison.InvariantCultureIgnoreCase))
    hearing = centreSession.StandardHearing;
else
    hearing = centreSession.NonStandardHearing;

hearing.SetOutcome(
    disciplineSvc.SaveResultA(hearing.ID, OffenceID, Outcome),
    disciplineSvc.SaveResultB(hearing.ID, txt1.Text, "ResultHearing"));

Some additional notes:

  1. I have also changed string.Compare() == 0 to string.Equals(), because I think it expresses the intent better.
  2. Some of the names are quite bad (e.g. txt1 or SaveResultA), you should improve those.
  3. Dealing with ids a lot is not very object-oriented. I think it would make more sense to deal directly with Outcome and OutcomeNote objects, not just their ids.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.