You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently each of our repositories implements IRepository, which in turn implements all other repository interfaces we currently have. This adds some inflexibility to endpoints which do not allow for some operations. Solutions are:
Leave IRepository as it is and add NotSupportedException to the repositories
Refactor IRepository into smaller interfaces and implement these interfaces on a need basis.
I advocate 2, not only are empty interfaces considered code smell to some degree, but our current pattern also violates the Interface segregation principle of SOLID.
In addition to refactoring out current interfaces, we should do some housekeeping. Current methods should either be be renamed to already established methods in C# (i.e. All(), Single(), etc.), or be renamed to CRUD method names (i.e. Get()).
Furthermore we also have to discuss, whether our code should fail-silent or not. Personally I advocate for a fail-fast approach. Therefore we should translate any HTTP-Statuscode into proper exceptions. As a curtesy to the user, we also could add fail-silent methods to the repositories (much akin to the ...OrDefault methods in LINQ).
Obviously every repository method is executed asynchronous and synchronous methods have been removed completely, since they serve no purpose anymore.
The current v2 repository layout looks like the code below:
public class ColorService : ServiceBase<ColorPalette>, IDiscoverService<int>, IApiService<int, ColorPalette>, ILocalizable
{
public ColorService(
HttpClient httpClient,
HttpResponseConverter responseConverter,
ICache<ColorPalette> cache,
IConverter<IEnumerable<int>, IEnumerable<int>> identifiersConverter,
IConverter<ColorPaletteDTO, ColorPalette> colorConverter)
: base(httpClient, responseConverter, cache)
{}
public CultureInfo Culture { get; set; }
public Task<IEnumerable<int>> DiscoverAsync() {}
public async Task<IEnumerable<int>> DiscoverAsync(CancellationToken cancellationToken) {}
public Task<IEnumerable<ColorPalette>> GetAsync(CancellationToken cancellationToken) {}
public Task<IEnumerable<ColorPalette>> GetAsync(CancellationToken cancellationToken, params int[] identifiers) {}
public async Task<IEnumerable<ColorPalette>> GetAsync(IEnumerable<int> identifiers, CancellationToken cancellationToken) {}
public async Task<ColorPalette> GetAsync(int identifier, CancellationToken cancellationToken) {}
public async Task<IEnumerable<ColorPalette>> GetAsync(Func<ColorPalette, bool> selector, CancellationToken cancellationToken) {}
}
public abstract class ServiceBase<T>
{
protected ServiceBase(HttpClient client, HttpResponseConverter responseConverter, ICache<T> cache)
{
this.Client = client;
this.ResponseConverter = responseConverter;
this.Cache = cache;
}
public HttpClient Client { get; }
public HttpResponseConverter ResponseConverter { get; }
public ICache<T> Cache { get; }
protected IEnumerable<int> CalculatePageSizes(int queryCount)
{
if (queryCount <= 200)
{
return new List<int> { queryCount };
}
return new List<int> { 200 }.Concat(this.CalculatePageSizes(queryCount - 200));
}
}
There's a question that pops into my mind for repositories with localized output. Should we deal with localization by setting a property on the repository, which sets the locale for all further requests until changed, or should we add a (optional) method parameters which only set the locale for the next request.
In my opinion the latter is the more sensible solution. The cache already does not care about the locale and stores everything. When accessing it, the user can either specify a locale or not. In the former case only results matching the locale are returned, in the latter case everything matching the filter is returned, regardless of the culture.
The HttpResponseConverter is a special convert, which does not inherit from IConverter<TIn, TOut>. It's ConvertAsync method accepts a HttpResponseMessage, an inner converter, an optional state and a Cancellation token. The method then deserializes the response into the appropriate data contract, before passing the results to the inner converter, which then converts the data contracts into actual objects.
The text was updated successfully, but these errors were encountered:
Currently each of our repositories implements
IRepository
, which in turn implements all other repository interfaces we currently have. This adds some inflexibility to endpoints which do not allow for some operations. Solutions are:IRepository
as it is and addNotSupportedException
to the repositoriesIRepository
into smaller interfaces and implement these interfaces on a need basis.I advocate 2, not only are empty interfaces considered code smell to some degree, but our current pattern also violates the Interface segregation principle of SOLID.
In addition to refactoring out current interfaces, we should do some housekeeping. Current methods should either be be renamed to already established methods in C# (i.e.
All()
,Single()
, etc.), or be renamed to CRUD method names (i.e.Get()
).Furthermore we also have to discuss, whether our code should fail-silent or not. Personally I advocate for a fail-fast approach. Therefore we should translate any HTTP-Statuscode into proper exceptions. As a curtesy to the user, we also could add fail-silent methods to the repositories (much akin to the
...OrDefault
methods in LINQ).Obviously every repository method is executed asynchronous and synchronous methods have been removed completely, since they serve no purpose anymore.
The current v2 repository layout looks like the code below:
There's a question that pops into my mind for repositories with localized output. Should we deal with localization by setting a property on the repository, which sets the locale for all further requests until changed, or should we add a (optional) method parameters which only set the locale for the next request.
In my opinion the latter is the more sensible solution. The cache already does not care about the locale and stores everything. When accessing it, the user can either specify a locale or not. In the former case only results matching the locale are returned, in the latter case everything matching the filter is returned, regardless of the culture.
The
HttpResponseConverter
is a special convert, which does not inherit fromIConverter<TIn, TOut>
. It'sConvertAsync
method accepts a HttpResponseMessage, an inner converter, an optional state and a Cancellation token. The method then deserializes the response into the appropriate data contract, before passing the results to the inner converter, which then converts the data contracts into actual objects.The text was updated successfully, but these errors were encountered: