C#•2y ago

❔ How can I map an API parameter to one of many specific classes that must provide a response?

I need to make an API that accepts a string parameter, tab_id specifying which settings tab to load. Each tab is handled by its own class, one class per page. E.g. the "general" settings tab is

GeneralSettingsTab : SettingsTab<GeneralSettingsTabData>

GeneralSettingsTab : SettingsTab<GeneralSettingsTabData>

Is there a better way than a switch expression to map tab_id to the specific class for that tab, because I don't like hard-coding the mappings? I'm calling the API from Angular, where my skills total 3 months experience, so I would prefer to just pass a string parameter to specify which SettingsTab derived service to use.

18 Replies

Unknown User•2y ago

Message Not Public

Brady Kelly•2y ago

I have this in my controller:

public SettingsController(IRequestContext requestContext, ISettingsService settingsService)
...
[HttpGet("PageModel")]
public async Task<SettingsViewModel> GetSettingsPageModel()
{
    return await _settingsService.GetSettings(_requestContext.User.Id, new ServiceExecutionContext(_requestContext));
}

public SettingsController(IRequestContext requestContext, ISettingsService settingsService)
...
[HttpGet("PageModel")]
public async Task<SettingsViewModel> GetSettingsPageModel()
{
    return await _settingsService.GetSettings(_requestContext.User.Id, new ServiceExecutionContext(_requestContext));
}

The settings service is injected by standard .NET DI:

public static IServiceCollection AddGeneraTabSettingsServices(this IServiceCollection services)
{
    return services
        .ThrowIfNull(nameof(services))
        .AddSingleton<ISettingsTab, GeneralSettingsTab>();
        // TODO Register all 10 ISettingsTab implementations.
}

public static IServiceCollection AddGeneraTabSettingsServices(this IServiceCollection services)
{
    return services
        .ThrowIfNull(nameof(services))
        .AddSingleton<ISettingsTab, GeneralSettingsTab>();
        // TODO Register all 10 ISettingsTab implementations.
}

I so far only have one only SettingsTab derived service, that is one ISettingsTab implementation:

internal class GeneralSettingsTab : SettingsTab<GeneralSettingsTabData>
{ 
    public override async Task<object> GetModelAsync(UserPreference[] preferences)
    {
        var data = new GeneralSettingsTabData
        {
            FontSizes = GetFontSizes(preferences), 
            ResultsPerPage = GetResultsPerPage(preferences)
            // Further populate the data object with settings provided my some injected services.
        };
    }
}

internal class GeneralSettingsTab : SettingsTab<GeneralSettingsTabData>
{ 
    public override async Task<object> GetModelAsync(UserPreference[] preferences)
    {
        var data = new GeneralSettingsTabData
        {
            FontSizes = GetFontSizes(preferences), 
            ResultsPerPage = GetResultsPerPage(preferences)
            // Further populate the data object with settings provided my some injected services.
        };
    }
}

pendramon•2y ago

If it makes sense to pass a complex data structure then its fine. For example when you need to send multiple pieces of data. Generally speaking its best to keep the data passed between the client and the server as simple as possible. This will make the API easier to use and help transmit less data over the network.

Brady Kelly•2y ago

Yeah, thanks, I have edited the part of my question regarding calling the API from Angular. I think a simple string parameter is the best

pendramon•2y ago

Where are you mapping the Id to a SettingsTab implementation? I don't even see tab_id mentioned in the code I may be blind though 😄

Brady Kelly•2y ago

I'm not mapping yet, and that is the goal of my question. The best idea I've had is to simply use a switch expression in the controller to map IDs to SettingsTab services, or maybe rather in a ServiceSelector class. So far the code I have posted is all I have, and it's not mine, it's inherited from another team, and it is hard-coded to only return on type of tab, hence there is not yet such a parameter. Eventually the end-point signature will be:

[HttpGet("PageModel")]
public async Task<SettingsViewModel> GetSettingsPageModel(string tab_id)

[HttpGet("PageModel")]
public async Task<SettingsViewModel> GetSettingsPageModel(string tab_id)

pendramon•2y ago

aha, gotcha Well you would have to specify what Id maps to what Implementation somewhere, there is no way around that part.

Brady Kelly•2y ago

Yeah, I'm basically only asking this to see if someone perhaps knows something less hard-code than a switch. something magic like maybe using the tab_id to determine which service to instantiate from the container, instead of using them all. But that will probably add too much complexity

pendramon•2y ago

Hmm, let me think Nah, I can't think of anything magic 😦 I think using a switch inside another service is your best bet.

Brady Kelly•2y ago

OK, thanks a lot for your time

pendramon•2y ago

I'd end up just doing something along the lines of this, all other ways I can think of are subpar or equal.

public sealed class SettingsTabLocator {
  private readonly IEnumerable<ISettingsTab> _settingsTabs;

  public SettingsLocator(IEnumerable<ISettingsTab> settingsTabs) {
    _settingsTabs = settingsTabs;
  }

  public ISettingsTab GetSettingsTabById(string tabId) {
    switch (tabId) {
      case "generalSettingsTab":
        return _settingsTabs.First(x => x is GeneralSettingsTab);
      case "otherSettingsTab":
        return _settingsTabs.First(x => x is OtherSettingsTab);
      default:
        throw new ArgumentException(nameof(tabId), "Invalid tab identifier.");
    }
  }
}

public sealed class SettingsTabLocator {
  private readonly IEnumerable<ISettingsTab> _settingsTabs;

  public SettingsLocator(IEnumerable<ISettingsTab> settingsTabs) {
    _settingsTabs = settingsTabs;
  }

  public ISettingsTab GetSettingsTabById(string tabId) {
    switch (tabId) {
      case "generalSettingsTab":
        return _settingsTabs.First(x => x is GeneralSettingsTab);
      case "otherSettingsTab":
        return _settingsTabs.First(x => x is OtherSettingsTab);
      default:
        throw new ArgumentException(nameof(tabId), "Invalid tab identifier.");
    }
  }
}

You can also inject IServiceProvider and use GetService but since these implementations are registered as singletons I'd rather not use an anti pattern.

Brady Kelly•2y ago

Thanks so much, I haven't used pattern matching much so I would not have thought to use it. I think it's a great idea

pendramon•2y ago

oops missed .First()

D.Mentia•2y ago

switches are usually always a pretty bad sign.
I'd start by making an enum for those Ids to usable names, then adding a static Dictionary<yourEnum, ISettingsTab> to hold/retrieve the tabs. They probably don't need to be generic

pendramon•2y ago

I had that same idea but wasnt sure if it would be better or just added complexity. For what reasons would you say thats better? Tryna learn myself here 😄

D.Mentia•2y ago

Mostly I just get triggered by switches, especially in generics because if you're switching on a generic's type, it's not a generic and/or has no reason to be I also don't know the performance of 'is', but I always thought it was reflectiony and not ideal But also now that I think about it, there's sortof a fundamental problem in even needing to know the type of the settings tab; you shouldn't care. That's sort of the concept behind an ISettingsTab, whatever's in it, you just call .ApplySettings Having a static dictionary might not be the right approach still because it would require the user to know what value corresponds to what SettingsTab, and you'd have to never change that, which is why the enum comes into play; exposing that to a user basically tells them. But it's not strongly typed so that's still probably not quite the best way to do it Oh, right. The best way to do it is actually really simple. Give the user a different endpoint for each type of SettingsTab they might want, if the SettingsTabs aren't generic enough to be interfaces or generic (without switches) Even if it is generic that's still probably the best way to do it. Each endpoint has its own model, as usual (I wonder if you can overload a single endpoint with different models, instead?), and if you have generics inside, you handle them yourself so the client doesn't have to 'know' anything.

  Task<SettingsViewModel> GetUserSettingsPageModel() => Settings<UserSettings>.GetModel();

  Task<SettingsViewModel> GetSomethingSettingsPageModel() => Settings<SomethingSettings>.GetModel();

  Task<SettingsViewModel> GetUserSettingsPageModel() => Settings<UserSettings>.GetModel();

  Task<SettingsViewModel> GetSomethingSettingsPageModel() => Settings<SomethingSettings>.GetModel();

(And really, each API endpoint should return its own specific model too, not each of them returning the same one) I, too, have trouble with the idea of all the duplicated properties across every model and class. But remember... you're supposed to reuse code. But this is structure, not code. You shouldn't try to have one endpoint do more than one thing in itself, but instead multiple endpoints that can do the thing, even if all the endpoints point at nearly the same code in the backend

Brady Kelly•2y ago

Doesn't having a separate endpoint for each tab imply duplicating 'fetch' functionality in the client? Where each tab has to do its own 'fetch', either that or I will still have to have a switch for them.

Accord•2y ago

Was this issue resolved? If so, run /close - otherwise I will mark this as stale and this post will be archived until there is new activity.

Gaming

Programming

❔ How can I map an API parameter to one of many specific classes that must provide a response?