current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

Is there a better way to write this in vs2010 C#?

public bool IsAccept()
{
    //check the status is accept
    if (Status == null)
        return false;
    return Status.ToLower() == "accept";
}

public bool IsRefer()
{
    //check the status is refer
    if (Status == null)
        return false;
    return Status.ToLower() == "refer";
}


public bool IsAnyReviewState()
{
    if (IsAccept() || IsRefer())
        return true;
    return false;
}

Maybe a simplified way in C# 4 which I'm still learning.

share|improve this question
    
not a real answer: just make it a ternary operator public bool IsAccept() { return (Status == null) ? false : Status.ToLower() == "accept"; } – sharp12345 Mar 9 '13 at 21:14

3 Answers 3

up vote 12 down vote accepted

A simpler version of your code (will work in all .NET versions down to 2.0) is the following:

public bool IsAccept
{
    get { return string.Equals("accept", Status, StringComparison.OrdinalIgnoreCase); }
}

public bool IsRefer
{
    get { return string.Equals("refer", Status, StringComparison.OrdinalIgnoreCase); }
}

public bool IsAnyReviewState
{
    get { return IsAccept || IsRefer; }
}

But a more appropriate solution would be to avoid string literals where an enumeration fits. If you receive the value from an external source, you should parse it into an enum as soon as possible, so that rest of the code deals with typed data only.

share|improve this answer
up vote 5 down vote

First wave

public bool IsAccept
{
    get { return Status != null && Status.ToLower() == "accept"; }
}

public bool IsRefer
{
    get { return Status != null && Status.ToLower() == "refer"; }
}

public bool IsAnyReviewState
{
    get { return IsAccept || IsRefer; }
}

Second - enums

The Status property shold be an anum or a type code class. With an enum, you will have a simple solution you just have to watch for some problems when using it: 0 is universal enum value, (SomeEnum)564564 is valid, even if the enum only contain 1, 2; always have a default in a switch statement when using enums.

Type code

Type code classes are sealed, they constructors are private and the available options are public static readonly fields. They are much like enums exept they can have other functions, like telling you IsReviewState() or you can easily localize them (you need a ResourceManager, a strongly typed resource file and for example a Display property to get the localized text).

share|improve this answer
up vote 0 down vote

Gave this answer on SO, posting here since the question wasn't migrated.

public bool IsAnyReviewState()
{
    return new [] {"accept", "refer"}.Contains((Status??string.empty).ToString().ToLower())
}
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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