3
\$\begingroup\$

This is what I have, But I think I can write it in a sohrter and more efficient way too, what do you suggest?

    string actionString= string.Empty;

    if (auditAction == DSRHelper.SaveStatusEnum.Save)
    {
        actionString = Properties.Resources.SAVE_ACTION_MESSAGE;
    }

    if (auditAction == DSRHelper.SaveStatusEnum.Complete && model.Dirty) 
    {
        actionString = "Save'NComplete";
    }
    else if(auditAction == DSRHelper.SaveStatusEnum.Complete)
    {
        actionString = "OnlyComplete.";
    }

EDIT: Ok I changed it a little bit, better now?

    string actionString= string.Empty;

    if (auditAction == DSRHelper.SaveStatusEnum.Save)
    {
        actionString = Properties.Resources.SAVE_ACTION_MESSAGE;
    }
    else if (auditAction == DSRHelper.SaveStatusEnum.Complete ) 
    {
        if (model.Dirty)
        {
            actionString = "Save'NComplete";
        }
        else
        {
            actionString = "OnlyComplete";
        }
    }
\$\endgroup\$
4
  • 1
    \$\begingroup\$ I really don't like the nested 'if' in the 'else if'. I would go with the method solution below. \$\endgroup\$ Commented Nov 8, 2012 at 22:37
  • \$\begingroup\$ I'm not very fond of switches... how about a stack ternary operators? XD \$\endgroup\$
    – Saturn
    Commented Nov 8, 2012 at 23:09
  • 1
    \$\begingroup\$ As @Omega suggested perhaps if not the switch then you could also look at the ternary I used for dirty part i.e. actionString = model.IsDirty ? "Save'NComplete" : "OnlyComplete" to further shorten it if you wanted \$\endgroup\$
    – dreza
    Commented Nov 9, 2012 at 1:54
  • \$\begingroup\$ I am not that fond of enums, they always mess the code with if's, switches. Can't you specialize your class and move this logic in each specialized class? \$\endgroup\$
    – Bhupendra
    Commented Nov 9, 2012 at 6:56

1 Answer 1

7
\$\begingroup\$

Maybe a switch? Although whether it's shorter or more efficient or even easier to read up for debate.

string actionString = GetStatusMessage(auditAction);

// ...
function GetStatusMessage(DSRHelper.SaveStatusEnum status) 
{
   switch(status)
   {
      case DSRHelper.SaveStatusEnum.Save:
           return Properties.Resources.SAVE_ACTION_MESSAGE;
      case DSRHelper.SaveStatusEnum.Complete:
           return model.Dirty ? "Save'NComplete" : "OnlyComplete.";
      default:
           return string.Empty;
   }
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I was 80% typing this exact same answer. One thing though, you might have to pass in the model object to the GetStatusMessage method. \$\endgroup\$ Commented Nov 8, 2012 at 22:36
  • \$\begingroup\$ @JeffVanzella yeah I thought about that after I wrote it and then figured I'd wait until the OP stated that it wasn't a class level variable. Just another way to skin this cat :) \$\endgroup\$
    – dreza
    Commented Nov 9, 2012 at 1:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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