3

I want to write if Else conditions in Shorter way in c# can you observe my code below it' so lengthy.. I want write in Shorter way , help me any one know. can we use is any other way to write it short ?

int paymentType;
if (lblPayment.Text == "Credit")
{
    paymentType = 1;
}
else if (lblPayment.Text == "Open Invoice")
{
    paymentType = 2;
}
else if (lblPayment.Text == "COD")
{
    paymentType = 3;
}
else if (lblPayment.Text == "Check")
{
    paymentType = 4;
}
else if (lblPayment.Text == "Paypal")
{
    paymentType = 5;
}
else if (lblPayment.Text == "Money Order")
{
    paymentType = 6;
}
else if (lblPayment.Text == "Other")
{
    paymentType = 7;
}
Xaruth
  • 4,034
  • 3
  • 19
  • 26
Bajid khan
  • 57
  • 5
  • Programming Rules :- If Multiple if-else (more than 5) in your code so avoid if-else and use switch case. if-else (more than 5) are increase programming complexity. – Anurag Jain Mar 03 '14 at 10:13
  • Use an enum `PaymentType` instead of an `int` to increase readability and safety. – Tim Schmelter Mar 03 '14 at 10:15

6 Answers6

15

Consider using a dictionary instead:

 Dictionary<string, int> lookup = new Dictionary<string, int>();
 lookup.Add("Credit", 1);
 lookup.Add("Open Invoice", 2);
 //etc

then: 

 int paymentType = lookup[lblPayment.Text];

This will throw a KeyNotFoundException if the value does not exist in the dictionary. If you want to supply a default value or do something else when the value does not exist in your lookup table you can use TryGetValue like so:

 int paymentType;
 if (lookup.TryGetValue(lblPayment.Text, out paymentType)) {
    //do stuff with paymentType
 } else {
    //handle error, paymentType is now 0.
 }
Bas
  • 26,772
  • 8
  • 53
  • 86
  • Nice, never though of doing something like this – DGibbs Mar 03 '14 at 10:10
  • If you don't want an exception if the key doesn't exist, you'll have to handle if with (for example) `if (!lookup.ContainsKey(lblPayment.Text)) ...` – Matthew Watson Mar 03 '14 at 10:12
  • 2
    @MatthewWatson: No, you'd use `TryGetValue` instead of using the indexer. (Or at least, that's what I'd do.) There's no need to look the key up twice. – Jon Skeet Mar 03 '14 at 10:12
  • +1 You could also improve the dictionary by using `Dictionary lookup = new Dictionary(StringComparer.CurrentCultureIgnoreCase);` – Tim Schmelter Mar 03 '14 at 10:20
  • @MatthewWatson Just a note: the `TryGetValue` method is faster than a `ContainsKey+Item`. There's an interesting question [here](http://stackoverflow.com/questions/9382681/what-is-more-efficient-dictionary-trygetvalue-or-containskeyitem). – Alberto Solano Mar 03 '14 at 10:20
5

Use a switch statement:

switch(lblPayment.Text)
{
    case "Credit":
     paymentType = 1;
     break;
    case "Open Invoice":
     paymentType = 2;
     break;
     ......
}
Tim Schmelter
  • 450,073
  • 74
  • 686
  • 939
Sadique
  • 22,572
  • 7
  • 65
  • 91
  • 3
    Why would you use a switch for this? It's just trying to map a string to a value - using either an array or a dictionary feels much cleaner than a switch here. – Jon Skeet Mar 03 '14 at 10:29
  • 2
    This can become just as unwieldly as a bunch of if statements. – Moo-Juice Mar 03 '14 at 10:40
3

You could use an enum:

public enum PaymentType
{
      NA,
      Credit,
      OpenInvoice,
      COD,
      Check,
      Paypal,
      MoneyOrder,
      Other
}

int paymentType = (int)PaymentType.Credit;

Edit to add a more concise explanation of how you might use this.

If you have control over the strings which you're comparing your label text to, you can do something like this:

int paymentType = (int)Enum.Parse(typeof(PaymentType), lblPayment.Text, true);

This assumes that you have enum friendly strings which is why I stipulated that you need control of them to avoid spaces/special characters etc... It looks like you aren't so all you would need to do is a lblPayment.Text.Replace(" ", string.Empty); before the parse to ensure the label Text matches the enum flags.

DGibbs
  • 14,316
  • 7
  • 44
  • 83
  • 1
    That doesn't show how you'd map from the string - in particular, you couldn't have an enum value name of `Money Order`, so you'll need some kind of extra mapping. – Jon Skeet Mar 03 '14 at 10:28
  • 1
    Absolutely, I'll leave that as an exercise for the OP as I'm not sure of where those strings are coming from and the level of control he has over them. – DGibbs Mar 03 '14 at 10:34
  • 1
    In that case this just doesn't answer the question at all. The question is about "how can I map strings to integers in a cleaner way than if/else". An answer of "use an enum" doesn't satisfy that. – Jon Skeet Mar 03 '14 at 10:39
  • _"I'm not sure of where those strings are coming from"_ Isn't that quite obvious? He's using `lblPayment.Text`. – Tim Schmelter Mar 03 '14 at 10:40
  • 1
    @TimSchmelter And how are those labels set? – DGibbs Mar 03 '14 at 10:48
  • @DGibbs: Is that important for the answer? They are not directly set by the user since a label is readonly. Apart from that you are just showing that OP could replace the `int` with an enum which is a good advice, but it does not answer the question how to replace the `if`s with something more concise. – Tim Schmelter Mar 03 '14 at 10:59
  • @TimSchmelter Of course it matters, I'll update the answer to demonstrate exactly why. Essentially, if he can control those strings and make them 'enum friendly' (no spaces or special chars etc..) then you can parse the value out of the string representation which matches the enum doing away with the if/else statements.. **See edit** – DGibbs Mar 03 '14 at 11:06
  • @DGibbs: but isn't that what Jon Skeet has already noticed? You cannot and should not rely that a string which is shown to the user can be converted to an `enum` directly. What if the user wants to see `"Money -Order"` for example? This will just bring up a new problem. You should not be "enum friendly" but user-friendly ;) – Tim Schmelter Mar 03 '14 at 11:14
  • @TimSchmelter Did you not see me agree with him? `What if the user wants to see "Money -Order" for example?` And that is why I asked whether he has control over those strings. I'm assuming he does so he would handle that case in code or use one of the other answers here which are perfectly fine. – DGibbs Mar 03 '14 at 11:22
1

just an idea , you can store this in an Array and at each index you store a value 

ExAr[1]="sometext";

I think this would be the shortest solution, you just call in 1 line to display a pre stored value 

string[] arr4 = new string[3];
arr4[0] = "one";
arr4[1] = "two";
arr4[2] = "three";

// Loop through all instances to find "one"
int i = 0;
while ((i = s.IndexOf('one', i)) != -1)
{

    // Print out the substring.
    Console.WriteLine(s.Substring(i));


    // Increment the index, the index is the payment value you seek
    i++;
}
Nour
  • 777
  • 1
  • 8
  • 25
  • It would be worth showing how you get from the string to the index (e.g. using IndexOf) – Jon Skeet Mar 03 '14 at 10:12
  • No, that's not really what you're after. I think you just want `string[] paymentTypes = { "", "Credit", "Open Invoice", ... }; int paymentType = paymentTypes.IndexOf(lblPayment.Text); if (paymentType != -1) { ... }` – Jon Skeet Mar 03 '14 at 10:28
1

Depending on what in particular you need to do in each of the condition branches, you can use several language constructs (switch, the ternary operator, ...). In your case you can use a lookup for the various types of money orders, mapping them to the payment types:

var paymentTypeMap = new Dictionary<string, int>
{
    { "Credit", 1 },
    { "Open Invoice", 2 },
    ...
}

int paymentType;
if (paymentTypeMap.TryGetValue(lblPayment.Text, out paymentType))
{
    // OK 
}
else
{
    // Unknown payment method
}
twoflower
  • 6,788
  • 2
  • 33
  • 44
-1

Switch.

Switch / Case can handle strings in C#, you know.

So that would be 

switch (lblPayment.Text)
case "Credit":
paymentType = 1;
break;
case "Open Invoice":
paymentType = 2;
break;

etc.etc.etc.

TomTom
  • 61,059
  • 10
  • 88
  • 148