skip to Main Content

I want to remove switch case block in GetById and Map methods to improve code readability and clean coding practices in my .NET Core app.

public enum IdentityProviderType
{
    Okta = 0,
    Ping = 1,
    Internal = 2
}

public async Task<UserResponseDTO> GetById(string id)
{
    IdentityProviderType provider = //custom logic to retrieve IdentityProviderType enum

    object idpUser = null;
    switch (provider)
    {
        case IdentityProviderType.Okta:
            idpUser = await _oktaUserService.GetById(id);
            break;
        case IdentityProviderType.Ping:
            idpUser = await _pingUserService.GetById(id);
            break;
        case IdentityProviderType.Internal:
            idpUser = await _internalUserService.GetById(id);
            break;
    }
    
    return _mapUserService.Map(idpUser);
}

public class MapUserService
{
    public UserResponseDTO Map(IdentityProviderType provider, object idpUser)
    {
        switch (provider)
        {
            case IdentityProviderType.Okta:
                var oktaUser = (OktaUser)idpUser;
                //Map OktaUser fields to UserResponseDTO fields
                break;
            case IdentityProviderType.Ping:
                var pingUser = (PingUser)idpUser;
                //Map PingUser fields to UserResponseDTO fields
                break;
        case IdentityProviderType.Internal:
                var internalUser = (InternalUser)idpUser;
                //Map InternalUser fields to UserResponseDTO fields
                break;
        }   
    }
}

From different resources on the internet regarding the same topic I came to possible idea but not sure how to implement it since I have different objects OktaUser/PingUser/InternalUser.

I started creating something like

public abstract class IdpUserService
{
    public abstract object GetById(string id);
}

But not sure how to complete it, should I use T or object as output?

Any help how to refactor and get rid of switch case would be helpful here.

4

Answers


  1. Maybe have only 1 user object with different properties instead of OktaUser, PingUser and InternalUser?

    Login or Signup to reply.
  2. It is hard to pick the best option without full overview of the codebase. Based on provided code the best option seems to be to abstract IdpUserService so it returns UserResponseDTO and handle conversions inside the concrete service implementation:

    public abstract class IdpUserService
    {
        public abstract UserResponseDTO GetById(string id);
    }
    

    If there are parts of code which require working with "raw" user then create mapping methods per type (either extension methods or as part of %SourceName%User types):

    public async Task<UserResponseDTO> GetById(string id)
    {
        IdentityProviderType provider = //custom logic to retrieve IdentityProviderType enum
        UserResponseDTO idpUser = provider switch
        {
            IdentityProviderType.Okta => (await _oktaUserService.GetById(id)).ToUserDto(),
            IdentityProviderType.Ping => (await _pingUserService.GetById(id)).ToUserDto(), 
            ...
            _ => throw new ArgumentOutOfRangeException()
        };
        return idpUser;
    }
    
    Login or Signup to reply.
  3. Something like this:

    public interface IIdentityProvider {
    
        Task<UserResponseDTO> GetById(string id);
    
        UserResponseDTO Map(object idpUser);
    
    }
    
    public static class ProviderFactory {
    
        public static IIdentityProvider GetIdentityProvider(IdentityProviderType providerType) {
            switch (providerType) {
                case IdentityProviderType.Okta:
                    return OktaUserService.Singleton;
                case IdentityProviderType.Ping:
                    return PingUserService.Singleton;
                case IdentityProviderType.Internal:
                    return InternalUserService.Singleton;
            }
            throw new NotSupportedException($"{nameof(IdentityProviderType)} '{providerType}' is not supported!");
        }
    
    }
    
    Login or Signup to reply.
  4. Here is the complete code that can help you.

    First, you will need to abstract the IdpUserService for method GetById.

    public abstract class IdpUserService
    {
        public abstract IdpUser GetById(string id);
    }
    

    And also you will need to abstract the IdpUser and move the method Map into it.

    public abstract class IdpUser
    {
        public abstract UserResponseDTO Map();
    }
    

    Now you can implement the specific version of the IdpUserService.

    public class OktaUserService : IdpUserService
    {
        public override IdpUser GetById(string id)
        {
            return new OktaUser();
        }
    }
    
    public class PingUserService : IdpUserService
    {
        public override IdpUser GetById(string id)
        {
            return new PingUser();
        }
    }
    
    public class InternalUserService : IdpUserService
    {
        public override IdpUser GetById(string id)
        {
            return new InternalUser();
        }
    }
    

    Similarly, you will need to implement specific versions of IdpUser to return from a specific service class.

    public class OktaUser:IdpUser
    {
        public override UserResponseDTO Map()
        {
            return new UserResponseDTO();
        }
    }
    
    public class PingUser:IdpUser
    {
        public override UserResponseDTO Map()
        {
            return new UserResponseDTO();
        }
    }
    
    public class InternalUser:IdpUser
    {
        public override UserResponseDTO Map()
        {
            return new UserResponseDTO();
        }
    }
    

    Now you have multiple implementations of IdpUserService from which you want to choose one using the enum IdentityProviderType.

    We will use dependency injection to construct objects. So we will register all the services to the DI container. And created one class NamedImpl to map the service to a specific IdentityProviderType.

    public static void Main(string[] args)
        {
            using IHost host = Host.CreateDefaultBuilder(args)
                .ConfigureServices((_, services) =>
                {
                    services.AddSingleton<OktaUserService>();
                    services.AddSingleton<PingUserService>();
                    services.AddSingleton<InternalUserService>();
                    services.AddSingleton<IdpUserServiceFactory>();
                    services.AddSingleton<NamedImpl<IdpUserService>>(_ =>
                    {
                        return new NamedImpl<IdpUserService>(Test2.IdentityProviderType.Okta.ToString(),
                            _.GetService<OktaUserService>());
                    });
                    services.AddSingleton<NamedImpl<IdpUserService>>(_ =>
                    {
                        return new NamedImpl<IdpUserService>(Test2.IdentityProviderType.Internal.ToString(),
                            _.GetService<InternalUserService>());
                    });
                    services.AddSingleton<NamedImpl<IdpUserService>>(_ =>
                    {
                        return new NamedImpl<IdpUserService>(Test2.IdentityProviderType.Ping.ToString(),
                            _.GetService<PingUserService>());
                    });
                    services.AddSingleton<Test2>();
                })
                .Build();
            var test2 = host.Services.GetService<Test2>();
            test2.GetById("");
            host.RunAsync().Wait();
        }
    

    Now to get specific IdpUserService we will create a factory class that get the list of all registered IdpUserService types through the constructor. And a get method which gets the IdentityProviderType as a parameter and returns the IdpUserService of specific implementation for supplied parameter of IdentityProviderType.

    public class IdpUserServiceFactory
    {
        private readonly List<NamedImpl<IdpUserService>> idpUserServices;
    
        public IdpUserServiceFactory(List<NamedImpl<IdpUserService>> idpUserServices)
        {
            this.idpUserServices = idpUserServices;
        }
    
        public IdpUserService Get(Test2.IdentityProviderType type)
        {
            return idpUserServices.Where(P => P.Name == type.ToString()).FirstOrDefault()?.Impl;
        }
    }
    

    Now the code you have posted in question can be structured as bellow to utilize the all changes that we have done above.

    public class Test2
    {
        private readonly IdpUserServiceFactory idpUserServiceFactory;
    
        public Test2(IdpUserServiceFactory idpUserServiceFactory)
        {
            this.idpUserServiceFactory = idpUserServiceFactory;
        }
    
        public enum IdentityProviderType
        {
            Okta = 0,
            Ping = 1,
            Internal = 2
        }
    
        public async Task<UserResponseDTO> GetById(string id)
        {
            IdentityProviderType
                provider = IdentityProviderType.Internal; //custom logic to retrieve IdentityProviderType enum
            IdpUserService idpUserService = this.idpUserServiceFactory.Get(provider);
            IdpUser idpUser = idpUserService.GetById(id);
            return idpUser.Map();
        }
    }
    

    I know it seems to be over complicated but this way your code would be very extendible as you are using many implementations for the same type of service.

    Implementation for NamedImpl

    public class NamedImpl<T>
    {
        public string Name { get; private set; }
        public T Impl { get; private set; }
    
        public NamedImpl(string name, T impl)
        {
            Name = name;
            Impl = impl;
        }
    }
    
    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search