Skip to content

Remove int-based shortcuts in generic signatures #1044

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bart-degreed opened this issue Aug 31, 2021 · 0 comments · Fixed by #1093
Closed

Remove int-based shortcuts in generic signatures #1044

bart-degreed opened this issue Aug 31, 2021 · 0 comments · Fixed by #1093

Comments

@bart-degreed
Copy link
Contributor

bart-degreed commented Aug 31, 2021

We currently provide shortcut definitions for various interfaces and classes (service, repo, definition) when the ID type is int. The proposed plan is to remove these extra types for the following reasons:

  • The type int is not the obviously most commonly used or preferred key type. More common for SQL databases are long or GUID. The most commonly used key type for MongoDB is string. So in general, int doesn't deserve the special attention it gets today.
  • It complicates IoC container setup and resolving of types. This affects both performance and debuggability. For example, when a resource service asks for a repository instance, we first check if the key type is int and then prefer the single-parameter interface to resolve from the IoC container, falling back to the double-parameter interface (see here). This works fine, as long as Repository<TResource> always inherits from Repository<TResource, TId>, which is not something JADNC can enforce. Likewise, when the developer forgets to make the single-parameter class implement the required interfaces, we silently resolve the wrong type. These problems are hard to diagnose. Finally, Resharper suggests "you can inject interface of base type" in constructor signatures (replacing IGetAllService<Article> with IGetAllService<Article, int>), which defeats the whole purpose of having these shortcut interfaces in the first place.
  • Given there are now two implementations for everything, extending them requires duplicate work. This can often be solved using inheritance, but not in all cases.

Given the above, we believe that being explicit on the key type is the overall better choice here. Note this is a breaking change. For example, users of JADNC will need to rewrite their controller from:

public sealed class TodoItemsController : JsonApiController<TodoItem>
{
    public TodoItemsController(IJsonApiOptions options, ILoggerFactory loggerFactory,
        IResourceService<TodoItem> resourceService)
        : base(options, loggerFactory, resourceService)
    {
    }
}

to:

public sealed class TodoItemsController : JsonApiController<TodoItem, int>
//                                                                    ^^^
{
    public TodoItemsController(IJsonApiOptions options, ILoggerFactory loggerFactory,
        IResourceService<TodoItem, int> resourceService)
//                                 ^^^
        : base(options, loggerFactory, resourceService)
    {
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

1 participant