Better way of rewriting below method

Question

I have below method that does work Okay but I believe we can make it better by some how getting rid of these if conditions, but not sure how?

 protected void SaveSession<T>(T sessionProperty, Management management) where T : class
        {
            string propertyType = typeof(T).Name;

            if (propertyType.Equals(typeof(A).Name))
            {
                management.A = sessionProperty as A;
            }
            else if (propertyType.Equals(typeof(B).Name))
            {
                management.B = sessionProperty as B;
            }
            else if (propertyType.Equals(typeof(C).Name))
            {
                management.C = sessionProperty as C;
            }

            Session["mysession"] = management;
        }

I am using latest C# version 7.0

Which C# version are you using? In C# 7 and later you can use a pattern matching `switch` statement — Panagiotis Kanavos, Jun 13 '19 at 15:29
Get an ArrayList property in Management. Filter with Enumerable.OfType() and add/set the sessionProperty variable. — Cleptus, Jun 13 '19 at 15:30
Sounds like a good place for the strategy pattern. Other than that, don't compare type names, just compare the types directly. — Alejandro, Jun 13 '19 at 15:34
@User the latest released version is 7.3. The latest *preview* version is 8.0. I assume you use C# 7.3, or at least C# 7? — Panagiotis Kanavos, Jun 13 '19 at 15:35
@Alejandro I am not been able to because one i type while other is class — User, Jun 13 '19 at 15:37

score 3 · Accepted Answer · answered Jun 13 '19 at 15:32

3

In C# 7 and later you can use a pattern matching switch statement, eg :

switch(sessionProperty)
{
    case A a:
        management.A=a;
        break;
    case B b:
        management.B=b;
        break;
    case C c:
        management.C=c;
        break;
}

In earlier versions you can use the is operator to check the type, eg :

if (sessionProperty is A)
{
    management.A=(A)sessionProperty;
}
else if (sessionProperty is B)
{
    management.B=(B)sessionProperty;
}

answered Jun 13 '19 at 15:32

Panagiotis Kanavos

120,703
13
188
236

sorry I am using I am using latest C# version 7.0 – User Jun 13 '19 at 15:36
If `Management` could be changed, I would suggest an `ArrayList` property in it + `.OfType()` – Cleptus Jun 13 '19 at 15:37
@User pattern matching works in C# 7 – Panagiotis Kanavos Jun 13 '19 at 15:37
@bradbury9 why do that? – Panagiotis Kanavos Jun 13 '19 at 15:37
To reduce the amount of properties in Management. Seems like a poor design – Cleptus Jun 13 '19 at 15:38
@bradbury9 there's nothing wrong with using only the properties you need, with the types you need. That's how all classes work. A weakly typed list of values on the other hand can cause a lot of problems. Besides, that's what `dynamic` and ExpandoObject are for – Panagiotis Kanavos Jun 13 '19 at 15:40
@bradbury9 using a list of weakly typed values means that someone down the line will have to write the same kind of code the OP wrote to try and *read* the values. – Panagiotis Kanavos Jun 13 '19 at 15:41
@PanagiotisKanavos nah, just `T GetData( return management.OfType().SingleOrDefault(); )`. It would be problematic if there were multiple properties of the same type indeed, but it depends on the use case. If properties were needed, you answer would be sweet. – Cleptus Jun 13 '19 at 15:45
@bradbury9 think what that code does for a moment. It enumerates a list, type checks, then returns the first item. Compare that to `Management.A`, possibly a *domain* entity, with properties that clearly show what it's for. On the one hand, an opaque list of values that requires iterations and assumptions on what each value does. On the other hand, a direct reference that costs nothing and clean code that's easy to read. That's the difference between dynamically and statically typed languages, only in the case of the list of values you don't even know what's in there – Panagiotis Kanavos Jun 13 '19 at 15:48

Better way of rewriting below method

1 Answers1