C#•10mo ago

Best practice to eliminate switch statements?

I have a terrible feeling this is bad:

switch (apiQueryType)
{
    case ApiListRequest.ServerInfo:
        apiReturn.DataObject = SingleDataObjectFromXDoc<Server>(xDoc, "serveurs");
        break;
    case ApiListRequest.UserInfo:
        apiReturn.DataObject = SingleDataObjectFromXDoc<User>(xDoc, "ssuser");
        break;
    case ApiListRequest.UserLevelsList:
        apiReturn.DataObject = MultipleDataObjectsFromXDoc<UserLevel>(xDoc, "userlevel");
        break;
    case ApiListRequest.GameInfoTypeList:
        apiReturn.DataObject = MultipleDataObjectsFromXDoc<Data.Entities.GameInfoType>(xDoc, "info");
        break;
    case ApiListRequest.RomInfoTypeList:
        apiReturn.DataObject = MultipleDataObjectsFromXDoc<Data.Entities.RomInfoType>(xDoc, "info");
        break;
    case ApiListRequest.SystemList:
        apiReturn.DataObject = MultipleDataObjectsFromXDoc<Data.Entities.System>(xDoc, "systeme");
        break;
    case ApiListRequest.NumberOfPlayersList:
        apiReturn.DataObject = MultipleDataObjectsFromXDoc<Data.Entities.NumberOfPlayers>(xDoc, "nbjoueur");
        break;
        //.... loads more of these...
    default:
        break;
}

switch (apiQueryType)
{
    case ApiListRequest.ServerInfo:
        apiReturn.DataObject = SingleDataObjectFromXDoc<Server>(xDoc, "serveurs");
        break;
    case ApiListRequest.UserInfo:
        apiReturn.DataObject = SingleDataObjectFromXDoc<User>(xDoc, "ssuser");
        break;
    case ApiListRequest.UserLevelsList:
        apiReturn.DataObject = MultipleDataObjectsFromXDoc<UserLevel>(xDoc, "userlevel");
        break;
    case ApiListRequest.GameInfoTypeList:
        apiReturn.DataObject = MultipleDataObjectsFromXDoc<Data.Entities.GameInfoType>(xDoc, "info");
        break;
    case ApiListRequest.RomInfoTypeList:
        apiReturn.DataObject = MultipleDataObjectsFromXDoc<Data.Entities.RomInfoType>(xDoc, "info");
        break;
    case ApiListRequest.SystemList:
        apiReturn.DataObject = MultipleDataObjectsFromXDoc<Data.Entities.System>(xDoc, "systeme");
        break;
    case ApiListRequest.NumberOfPlayersList:
        apiReturn.DataObject = MultipleDataObjectsFromXDoc<Data.Entities.NumberOfPlayers>(xDoc, "nbjoueur");
        break;
        //.... loads more of these...
    default:
        break;
}

How would you go about refactoring it. My basic thinking is to create a class which contains the string, the apiReturn enum, the Object type (how would you do this?) and something to help any function to call the right method (one of two: SingleDataObjectFromXDoc or MultipleDataObjectsFromXDoc)

8 Replies

Angius•10mo ago

You could easily use a switch expression here

apiReturn.DataObject = apiQueryType switch {
    ApiListRequest.ServerInfo => SingleDataObjectFromXDoc<Server>(xDoc, "serveurs"),
    ApiListRequest.UserInfo => SingleDataObjectFromXDoc<User>(xDoc, "ssuser"),
    // ...
    _ => throw new OutOfRangeException() // or some other one
};

apiReturn.DataObject = apiQueryType switch {
    ApiListRequest.ServerInfo => SingleDataObjectFromXDoc<Server>(xDoc, "serveurs"),
    ApiListRequest.UserInfo => SingleDataObjectFromXDoc<User>(xDoc, "ssuser"),
    // ...
    _ => throw new OutOfRangeException() // or some other one
};

Or, sure, you could extract some of it to a method, even to a local function like

WhateverType Data<T>(string str)
    => SingleDataObjectFromXDoc<T>(xDoc, str);

apiReturn.DataObject = apiQueryType switch {
    ApiListRequest.ServerInfo => Data<Server>("serveurs"),
    ApiListRequest.UserInfo => Data<User>("ssuser"),
    // ...
    _ => throw new OutOfRangeException() // or some other one
};

WhateverType Data<T>(string str)
    => SingleDataObjectFromXDoc<T>(xDoc, str);

apiReturn.DataObject = apiQueryType switch {
    ApiListRequest.ServerInfo => Data<Server>("serveurs"),
    ApiListRequest.UserInfo => Data<User>("ssuser"),
    // ...
    _ => throw new OutOfRangeException() // or some other one
};

stigzlerOP•10mo ago

Like the first one - a bit tidier! I was wondering whether there's a way to store the 'variables' in each switch statement in a constant and then just do the call via two lines. E.g. in psuedo-code:

public class AipListRequestType
{
  ApiListRequest RequestType {get; set;}
  string ElementName {get; set;}
  T ObjectType {get; set;}
  bool MultiObjectSearch {get; set;}
}

public class Main
{
  // some function to populate List<AipListRequestTypes> e.g.
  List<AipListRequestTypes> AipListRequestTypesList = new List<AipListRequestTypes> 
  {RequestType = ApiListRequest.ServerInfo, ObjectType = Server, MultiObjectSearch = false //etc }
  {RequestType = ApiListRequest.SystemList, ObjectType = System, MultiObjectSearch = true //etc }
  
  // and then, somewhere in code:
  ApiListRequest passedSearchType = {blah}
  AipListRequestType requestType = AipListRequestTypesList.Find(x=> x.RequestType  = passedSearchType).First()
  if (requestType.MultiObjectSearch)
    {apiReturn.DataObject = MultipleDataObjectsFromXDoc<requestType.ObjectType>(xDoc, requestType.ElementName);}
  else
    {apiReturn.DataObject = SingleDataObjectFromXDoc<requestType.ObjectType>(xDoc, requestType.ElementName);}
}

public class AipListRequestType
{
  ApiListRequest RequestType {get; set;}
  string ElementName {get; set;}
  T ObjectType {get; set;}
  bool MultiObjectSearch {get; set;}
}

public class Main
{
  // some function to populate List<AipListRequestTypes> e.g.
  List<AipListRequestTypes> AipListRequestTypesList = new List<AipListRequestTypes> 
  {RequestType = ApiListRequest.ServerInfo, ObjectType = Server, MultiObjectSearch = false //etc }
  {RequestType = ApiListRequest.SystemList, ObjectType = System, MultiObjectSearch = true //etc }
  
  // and then, somewhere in code:
  ApiListRequest passedSearchType = {blah}
  AipListRequestType requestType = AipListRequestTypesList.Find(x=> x.RequestType  = passedSearchType).First()
  if (requestType.MultiObjectSearch)
    {apiReturn.DataObject = MultipleDataObjectsFromXDoc<requestType.ObjectType>(xDoc, requestType.ElementName);}
  else
    {apiReturn.DataObject = SingleDataObjectFromXDoc<requestType.ObjectType>(xDoc, requestType.ElementName);}
}

bloody hell - should have just coded it directly to see if it works

Angius•10mo ago

No, you can't store generic parameters in a variable

var something = IdkSomeType;
Foo<something>();

var something = IdkSomeType;
Foo<something>();

will not work

stigzlerOP•10mo ago

I did wonder - being T ObjectType {get; set;} in this case? or the MultipleDataObjectsFromXDoc<requestType.ObjectType>?

Angius•10mo ago

No, generic parameters cannot take variables

stigzlerOP•10mo ago

ah - so whichever way you cut it - this would need some kind of switch statement? Thanks zzzzzzz - you're always so helpful

Jimmacle•10mo ago

one thing you could do is use a switch expression instead, the syntax is more ergonomic for this kind of code ree, chat jumped a mile ignore me hate it when the post is just big enough to hide that someone already responded :KEKW:

stigzlerOP•10mo ago

🤣 No worries - thanks for chipping in!

Gaming

Programming

Best practice to eliminate switch statements?