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
Maybe have only 1 user object with different properties instead of
OktaUser
,PingUser
andInternalUser
?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 returnsUserResponseDTO
and handle conversions inside the concrete service implementation: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):Something like this:
Here is the complete code that can help you.
First, you will need to abstract the
IdpUserService
for methodGetById
.And also you will need to abstract the
IdpUser
and move the methodMap
into it.Now you can implement the specific version of the
IdpUserService
.Similarly, you will need to implement specific versions of
IdpUser
to return from a specific service class.Now you have multiple implementations of
IdpUserService
from which you want to choose one using the enumIdentityProviderType
.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 specificIdentityProviderType
.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 theIdentityProviderType
as a parameter and returns theIdpUserService
of specific implementation for supplied parameter ofIdentityProviderType
.Now the code you have posted in question can be structured as bellow to utilize the all changes that we have done above.
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