0

Can I replace this code snippent with a C#8 switch expression?

Note that if ObjectType is Computer, ObjectClass will contain "person" so ordering matters.

Also, the question is academic and I am only interested in the switch expression and not how to solve this particular problem.

public List<string> ObjectClass { get; set; }
public ObjectType ObjectType {
    get {
        if (ObjectClass.Contains("group"))      { return ObjectType.Group; }
        if (ObjectClass.Contains("computer"))   { return ObjectType.Computer; }
        if (ObjectClass.Contains("person"))     { return ObjectType.User; }
        return ObjectType.Unknown;
    }
}
Iliar Turdushev
  • 4,935
  • 1
  • 10
  • 23
bob
  • 579
  • 1
  • 8
  • 22
  • 1
    It's a switch *expression* - statements don't return anything, only expressions do. Yes you can, but it won't be much prettier. The operations you posted search through that list multiple times too, so you should be looking for ways to *avoid* multiple iterations. You probably need a LINQ call to return the first occurence of any of those three words and map it to the target value. That would result in a single iteration only, making the code up to 300% faster – Panagiotis Kanavos Aug 27 '20 at 17:04
  • Is `ObjectType` an Enum? In that case you could use `Enum.Parse()` to parse any data found directly to an `ObjectType` – Panagiotis Kanavos Aug 27 '20 at 17:06
  • 2
    As @PanagiotisKanavos noted using `switch expression` would result in not a pretty code. Here is demo how you can rewrite your getter using `switch expression`: https://dotnetfiddle.net/W57TEO. In my opinion using `switch expression` is not a good approach in this case, because **1)** there is no value against which matching is performed; **2)** each `case branch` does not contain matching value. – Iliar Turdushev Aug 27 '20 at 17:15
  • @IliarTurdushev thanks. The question is academic. I read about the new switch earlier and wanted to see if I could get it to work here. I will mark it as the answer if you want to repost. – bob Aug 27 '20 at 19:04
  • @PanagiotisKanavos The ObjectClass list is messy. There are only 2 - 5 values on the list so performance isn't a problem. Also, there are overlapping values between them and the "correct" value may not be first or last (i.e. a computer is a person so they overlap 100%). But yes, I should have said "expression" – bob Aug 27 '20 at 19:07
  • @bob your code picks the first match in a specific order (group, computer, person). No overlap is considered. Using switch results in *a lot more, less clear* code so you get no benefit from it. With LINQ you could write `ObjectClass.Select(x=>Enum.TryParse(x,true,out var t)?t:ObjectType.Unkown).OrderBy(x=>x).First()`, assuming `ObjectType` is an Enum and `Unknown` has the largest value. Post `ObjectType` – Panagiotis Kanavos Aug 28 '20 at 07:11
  • 1
    `the question is academic and I am only interested in the switch expression` that's a very bad example then. It's one of the cases where it's cleaner to *not* use either switch expressions or switch statements – Panagiotis Kanavos Aug 28 '20 at 07:13

2 Answers2

0

This answer builds off the solution provided by @IliarTurdushev and credit should go to him. Also refactored using suggestion from @NetMage, with thanks.

public class Program
{
    public  List<string> ObjectClass { get; set; }
    public  ObjectType sample
    {
        get => ObjectClass switch {
            _ when ObjectClass.Contains("group") => ObjectType.Group,
            _ when ObjectClass.Contains("computer") => ObjectType.Computer,
            _ when ObjectClass.Contains("person") => ObjectType.Person,
            _ => ObjectType.Unknown};
    }

    public enum ObjectType
    {
        Group = 1,
        Computer = 2,
        Person = 3,
        Unknown = 4
    }
}
TheLeb
  • 144
  • 1
  • 11
  • 1
    This is no better than the original code - it scans the list 3 times – Panagiotis Kanavos Aug 28 '20 at 07:03
  • 2
    But answers the question asked by Bob. Please note he said, "Also, the question is academic and I am only interested in the switch expression and not how to solve this particular problem." – TheLeb Aug 28 '20 at 12:49
  • The answer here is "don't do that". This is actually used as a *counter-example* by F# folks to show how limited C#'s pattern matching is – Panagiotis Kanavos Aug 28 '20 at 12:56
  • Then you are welcome to downvote the original question. Thanks and have a great day! – TheLeb Aug 28 '20 at 12:58
  • I would suggest using `get => ObjectClass switch`... instead of a full body - that is the point of using an expression. – NetMage Aug 28 '20 at 21:10
  • What would that code look like NetMage? And what is the point of using an expression? Please finish. – TheLeb Aug 31 '20 at 12:10
  • `get => ObjectClass switch { _ when ObjectClass.Contains("group") => ObjectType.Group, _ when ObjectClass.Contains("computer") => ObjectType.Computer, _ when ObjectClass.Contains("person") => ObjectType.Person, _ => ObjectType.Unknown };` – NetMage Aug 31 '20 at 19:09
0

You could combine LINQ and switch to have a (semi) efficient version:

public ObjectType type
{
    get => ObjectClass.Select(c => c switch { "group" => ObjectType.Group, "computer" => ObjectType.Computer, "person" => ObjectType.Person, _ => (ObjectType?)null})
                      .FirstOrDefault(t => t != null) ?? ObjectType.Unknown;
}
NetMage
  • 26,163
  • 3
  • 34
  • 55
  • No reason for this switch, the same can be done with a case-insensitive `Enum.TryParse` – Panagiotis Kanavos Aug 31 '20 at 12:58
  • @PanagiotisKanavos True, but this question was specifically about using the C# 8.0 `switch` expression. – NetMage Aug 31 '20 at 19:11
  • Which simply isn't necessary, something demonstrated by this answer. The example simply isn't good. BTW the way the `if` statements are ordered, `group` takes precedence over `computer` etc. `FirstOrDefault` isn't enough, you'd need to order them as well. If the values come from an enum, this could be as easy as `OrderBy(x=>x)` – Panagiotis Kanavos Sep 01 '20 at 06:41
  • @PanagiotisKanavos They are ordered that way specifically because in AD, all `computer` accounts are also `person` accounts (there's a bug in .Net query by example with subclasses because of this) so you must check for `computer` first. – NetMage Sep 01 '20 at 18:18