In my model, I have things called Reports that have known workflow associated with them.

I've got a requirement to output at which state a report currently is.

public abstract partial class Report{
   public virtual string StateStatus(){
     if (Check.IsSuccessful) return "Approved";
     if (Check.AreClarificationsSent) return "Clarifications";
     if (Check.IsInProgress) return "Check in progress";
     if (IsTemplateSent) return "Sent";
     if (IsReceived) return "Received";
     return "Not received";

I dislike this method. It won't break, but leaves some nausea in my head, just doesn't feel right.

Got any recommendations for improvement?

  • \$\begingroup\$ Are those calculated properties you are checking? \$\endgroup\$ Commented Aug 4, 2011 at 13:53
  • \$\begingroup\$ @ChaosPandion For calculating something, I'm always using methods. Properties are static, with already known values. \$\endgroup\$ Commented Aug 4, 2011 at 14:39
  • \$\begingroup\$ can it so happen, that all Check.xxx properties are true? If not is there a logic, which implements radio-button-like functionality - i.e. changing one of these changes the rest? \$\endgroup\$
    – Sunny
    Commented Aug 4, 2011 at 16:23
  • \$\begingroup\$ @Sunny No, Check.IsSuccessful and Check.IsInProgress are mutually exclusive. However - Check.IsInProgress and Check.AreClarificationsSent can be true simultaneously. StateStatus() method should give priority to "Clarifications". What exactly that changes, what are You thinking? \$\endgroup\$ Commented Aug 4, 2011 at 20:57
  • \$\begingroup\$ I was thinking for a Status property, which changes whenever some of these change - i.e. move the proper logic where it belongs. It could be not good fit for you. But check my answer for another possible solution. \$\endgroup\$
    – Sunny
    Commented Aug 5, 2011 at 13:07

4 Answers 4


It seems that you are wanting your Check enum to be able to provide a little more information for you without the messiness of the if-then clutter. What about combining the status strings with the enumerator? Something like:

public class CheckStatus{
    public static readonly CheckStatus 
      IsSuccessful = new CheckStatus { Value = "Approved" },
      AreClarificationsSent = new CheckStatus { Value = "Clarifications" },
      IsInProgress = new CheckStatus { Value = "Check in progress" },
      IsTemplateSent = new CheckStatus { Value = "Sent" },
      IsReceived = new CheckStatus { Value = "Received" },
      NotReceived  = new CheckStatus { Value = "Not Received" };

    private CheckStatus() { }
    public string Value{ get; private set; }

Very easy to maintain and puts the minor implementation details behind the purpose of the Check enumerator. Provides much the same benefit of an enumeration. I would have liked to make this a struct, but I would not (without going into IL) have a parameterless private ctor.

  • \$\begingroup\$ This ain't bad stuff. Using class enums myself. But that is still enum which couples all these vaguely related processes together too tightly. Also - Check is object for itself - it is responsible for checking [un]success of report. IsTemplateSent, IsReceived flags belong to report not its check (report template is generated, then - received filled out and that triggers report check to be available). It makes no sense for Check to be responsible for state of Report. \$\endgroup\$ Commented Aug 4, 2011 at 20:52
  • \$\begingroup\$ @Arnis L. - Now what if we renamed it CheckStatus and attached the instance to the Check object? I would also rename the instance property to Value. \$\endgroup\$ Commented Aug 4, 2011 at 20:58
  • \$\begingroup\$ @ChaosPandion IsTemplateSent belongs to report not check. It is not status of check. \$\endgroup\$ Commented Aug 5, 2011 at 7:59
  • \$\begingroup\$ @Arnis: Both, IsTemplateSent and IsReceived looked like bool members of Report. Remove those from CheckStatus if you like. In your original sample, the problem is that the status returned by IsTemplateSent will overwrite any return value from CheckStatus. The resulting values "Received" and "Not Received" do make sense as return status values - obviously, you can make whatever changes you like. \$\endgroup\$
    – IAbstract
    Commented Aug 5, 2011 at 12:11

I've used StringEnum as defined here several times and it works quite nicely. It works much like IAbstract's solution, but without needing to implement a separate class.

Then, wherever you need to reference the StringValue you can just use

enum Check 
  [StringValue("Check in progress")] InProgress,
  [StringValue("Not yet received")] NotReceived  

Check c = Check.InProgress;
string text = StringEnum.GetStringValue(c);

To avoid expansive rewrites you may want to use this approach.


A state machine. This tidies up all of those messy methods into a single property which stores the current state of your object. The downside to this is you'll need to tidy up your other objects, too...

public partial class Report {

    public enum State {
        NotReceived = 0,
        Received = 1,
        TemplateSent = 2,
        InProgress = 3,
        ClarificationSent = 4,
        Successful = 5

    public State Status { get; protected set; }
  • \$\begingroup\$ Have thought about this. #1 InProgress means that report check is in progress and not report itself. I don't want to pull out knowledge about check outside of it. #2 this increases coupling. I really don't want to think about sending report template and processing clarifications simultaneously that this enum would force me to. So I kind a dislike this approach equally much. \$\endgroup\$ Commented Aug 4, 2011 at 15:07

Add check state enum and create Check.State property.

Same for the report.

Then your method will look like (pseudo):

    var ret = report.State.ToString();
    if (Check.State != CheckState.Unprocessed)
        ret = Check.State.ToString();

    return ret;

or (ugly):

return (Check.State == CheckState.Unprocessed) ? State.ToString() : Check.State.ToString();

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.