2

I have the following code:

if (testQuestion.Result == "t") { testQuestion.CorrectCount++; }
if (testQuestion.Result == "f") { testQuestion.IncorrectCount--; }
if (testQuestion.Result == "s") { testQuestion.ShownCount++; }

Is there a way I can remove the need for the three if statements ?

Soner Gönül
  • 97,193
  • 102
  • 206
  • 364
  • 2
    You can use [`switch` statement](http://msdn.microsoft.com/en-us/library/06tc147t.aspx) instead of multiple if statements. Read http://stackoverflow.com/questions/395618/is-there-any-significant-difference-between-using-if-else-and-switch-case-in-c – Soner Gönül Mar 16 '14 at 14:08
  • Can you switch on a string in c#? (Granted: you could switch on a character in this case though) – Bathsheba Mar 16 '14 at 14:10
  • 2
    Actually you can switch strings in C#. – Vlad Schnakovszki Mar 16 '14 at 14:13
  • 1
    Is your `Result` property of type `Char` or `String`? – Crono Mar 16 '14 at 14:34
  • @Crono double quote warrants a string type, isn't it? Curious to what you would suggest if char? – Baljeetsingh Sucharia Mar 16 '14 at 15:43
  • 1
    @BaljeetsinghSucharia indeed it does, but since values being checked are one-char only I would have suggested to change `Result` property into a `Char` instead. Not that it will be *that* much more efficient but still, for the sake of best practices, and since the OP is obviously learning the language... thought it might be useful advice. :) – Crono Mar 16 '14 at 15:47
  • @Crono ahaa right, indeed a valuable suggestion on choosing right type. – Baljeetsingh Sucharia Mar 16 '14 at 15:50

5 Answers5

4

Since C# allows switching strings, you could use switch statements as follows:

switch (testQuestion.Result) {
    case "t": testQuestion.CorrectCount++; break;
    case "f": testQuestion.IncorrectCount--; break;
    case "s": testQuestion.ShownCount++; break;
}

You can find more details about the switch statements in C# here.

Vlad Schnakovszki
  • 8,434
  • 6
  • 80
  • 114
  • It will result in a compiler error if a break is missing. C# only allow fallthrough if the case is empty. – MAV Mar 16 '14 at 14:20
  • I think switch statement would be better if it were inside a method of the class such as testquestion.ProcessResult(). – Chad McGrath Mar 16 '14 at 14:37
4

You can use a switch statement:

switch (testQuestion.Result)
{
    case "t":
        testQuestion.CorrectCount++;
        break;
    case "f":
        testQuestion. IncorrectCount--;
        break;
    case "s":
        testQuestion.ShowCount++;
        break;
    default:
        // Result is a different value from what's expected
}

Or, if you prefer a more compact formula:

var q = testQuestion;
switch (q.Result)
{
    case "t": q.CorrectCount++; break;
    case "f": q. IncorrectCount--; break;
    case "s": q.ShowCount++; break;
}

I should mention though that if your Result property is of type Char then you should use apostrophes rather than quotation marks around your values.

EDIT: you may also want to have a default statement to handle unexpected cases. I just added it to the first block of code above.

Crono
  • 10,211
  • 6
  • 43
  • 75
  • I think switch statement would be better if it were inside a method of the class such as testquestion.ProcessResult(). – Chad McGrath Mar 16 '14 at 14:37
  • @ChadMcGrath actually I think the `Question` should be just that: a question. It shouldn't be responsible for tracking apparitions and good or bad answers. But at this point it's up to the OP to decide what's best. :) – Crono Mar 16 '14 at 14:41
3

An alternative would be to use the ternary operator:

testQuestion.CorrectCount += (testQuestion.Result == "t"? 1 : 0);
testQuestion.IncorrectCount += (testQuestion.Result == "f"? -1 : 0);
testQuestion.ShownCount += (testQuestion.Result == "s"? 1 : 0);
Vlad Schnakovszki
  • 8,434
  • 6
  • 80
  • 114
Baljeetsingh Sucharia
  • 1,990
  • 1
  • 19
  • 37
0

Option 2

You can wrap the logic in property as in this code. IMO, this is bit more readable.

public class TestQuestion
{
    private string _result;
    public string Result
    {
        get
        {
            return _result;
        }
        set
        {
            _result = value;
            this.CorrectCount += (this._result == Meta.correctResult ? Meta.incrementCount: 0);
            this.IncorrectCount += (this._result == Meta.incorrectResult ? Meta.decrementCount : 0);
            this.ShownCount += (this._result == Meta.shownResult ? Meta.incrementCount : 0);
        }
    }
    public int CorrectCount;
    public int IncorrectCount;
    public int ShownCount;
}

Here is code for Meta , this makes it easy to configure and control at one place.

public class Meta
{
    public static  string correctResult = "t";
    public static string incorrectResult = "f";
    public static string shownResult = "s";
    public static int incrementCount = 1;
    public static int decrementCount = -1;
}

This is how I used it in LinqPad

void Main()
{
    TestQuestion testQuestion = new TestQuestion();
    testQuestion.Result = "t";  
    testQuestion.Result = "f";  
    testQuestion.Result = "s";
    testQuestion.Dump();

}

Isn't it more readable ?

Baljeetsingh Sucharia
  • 1,990
  • 1
  • 19
  • 37
  • I strongly disagree with this design. Setting the `Result` property more than once - even with the same value - may completely unbalance increments. – Crono Mar 17 '14 at 17:40
  • @Crono you don't agree to design ? or you think use of design that might end up in unwanted result ? think of 2 ways , 1) what if business requirement says update the counter every-time value is assigned ? 2) updated the counter only when required and independent of value assignment (this is what i think you are referring ) what you think ? isn't that depends . lastly to be clear, intention here is to share views, learn and grow. No where I am intending to take sides of above answer. – Baljeetsingh Sucharia Mar 17 '14 at 19:04
  • I absolutely agree with your last statement and this is why I haven't voted this answer down, because regardless of what I think of the design that *might* still work (although I don't see why you'd set three different `Result` values in the `Main` method consecutively???). That being said, I still see lots of code smells in this - enough so that the OP should feel concerned about doing something like this. Your other answer wasn't perfect either but I still much prefer it over that one. – Crono Mar 17 '14 at 19:20
  • use in main is just for demo that we kind of get away from if in main stream and they are in side the class.. – Baljeetsingh Sucharia Mar 17 '14 at 19:42
  • i would wish to setup some time to go over with you on what you think is better / best answer and why ? also discuss the code smell. – Baljeetsingh Sucharia Mar 17 '14 at 19:43
0

Though late to the party, still few cents from me. Create a lookup through dictionary Dictionary<string,Action> with string value as key and the various operations in the action delegate. Then initialize that lookup in constructor. Below is the sample code.

 public class Example
 {
     private static Dictionary<string,Action> _lookup=new();
     private static void Init(TestQuestion testQuestion)
     {
         _lookup.Add("t",()=> testQuestion.CorrectCount++);
         _lookup.Add("f",()=> testQuestion.InCorrectCount--);
         _lookup.Add("s",()=> testQuestion.ShownCount++);
      }
      public void ExampleMethod(TestQuestion testQuestion)
      {
             Init(testQuestion);
             _lookup[testQuestion.Result]();    // this pull the matching action and execute with `()` added 
      }
}

This is a sample, in actual this lookup should be moved in separate class. The above code does away with multiple if-else

Ajeet Kumar
  • 687
  • 6
  • 12