4

Lets say I design a generic read only repository IReadOnlyRepository.

public interface IReadOnlyRepository
{
    ...    
    TEntity Find<TEntity>(object id) where TEntity : IEntity;
    IEnumerable<TEntity> Filter<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
    int Count<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
    ...
}

I already have a code base with several synchronous implementations of IReadOnlyRepository and want to extend my type (interface and it's implementations) with asynchronous versions of the same methods since today it is cool to async all the way.

public interface IReadOnlyRepository
{
    ...
    Task<TEntity> FindAsync<TEntity>(object id) where TEntity : IEntity;
    Task<IEnumerable<TEntity>> FilterAsync<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
    Task<int> CountAsync<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
}

This may sound subjective, but ...

Should I add async methods to the same file in a #region Async?

public interface IReadOnlyRepository
{
    ...    
    #region Sync

    TEntity Find<TEntity>(object id) where TEntity : IEntity;
    IEnumerable<TEntity> Filter<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
    int Count<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;

    #endregion

    #region Async

    Task<TEntity> FindAsync<TEntity>(object id) where TEntity : IEntity;
    Task<IEnumerable<TEntity>> FilterAsync<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
    Task<int> CountAsync<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;

    #endregion
    ...
}

... or should I make my repository partial and add async methods to partial files of the same type?

public partial interface IReadOnlyRepository
{
    TEntity Find<TEntity>(object id) where TEntity : IEntity;
    IEnumerable<TEntity> Filter<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
    int Count<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
}

public partial interface IReadOnlyRepository
{
    Task<TEntity> FindAsync<TEntity>(object id) where TEntity : IEntity;
    Task<IEnumerable<TEntity>> FilterAsync<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
    Task<int> CountAsync<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
}

... or should I add my async methods to a separate IRepositoryAsync interface?

public interface IReadOnlyRepository
{
    TEntity Find<TEntity>(object id) where TEntity : IEntity;
    IEnumerable<TEntity> Filter<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
    int Count<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
}

public interface IReadOnlyRepositoryAsync
{
    Task<TEntity> FindAsync<TEntity>(object id) where TEntity : IEntity;
    Task<IEnumerable<TEntity>> FilterAsync<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
    Task<int> CountAsync<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
}


public class EfRepository : IReadOnlyRepository, IReadOnlyRepositoryAsync

... or should I get rid of sync methods at all and go async only?

public interface IReadOnlyRepository
{
    Task<TEntity> FindAsync<TEntity>(object id) where TEntity : IEntity;
    Task<IEnumerable<TEntity>> FilterAsync<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
    Task<int> CountAsync<TEntity>(Expression<Func<TEntity, bool>> filter) where TEntity : IEntity;
}

What are the best practices of composing large types with both sync and async methods in a matter of file structure, code readability and reusability?

migrated from codereview.stackexchange.com Dec 28 '18 at 0:31

This question came from our site for peer programmer code reviews.

4

If you're going the async route, you want consumers of your interface to use the async version in order to realize any benefit. So it makes most sense to just get rid of the sync methods from the interface (the only reason not to would be to maintain backward compatibility).

If some of your implementations are synchronous, you can just use Task.FromResult to wrap the return value in a Task without any async code.

4

Should I add async methods to the same file in a #region Async?

Never. #region is a bandaid. You're better off taking the time and refactoring things well, or splitting them off into components. Even if you add it to the existing class, don't use #region - it hides the problem and prevents others (or future-you) from dealing with it.

... or should I make my repository partial and add async methods to partial files of the same type?

No, for similar reasons to #region unless you're going to use code generation to make the async implementations (or sync versions). I don't recommend that, but I can at least imagine scenarios where it's plausible.

... or should I add my async methods to a separate IRepositoryAsync interface?

Maybe. Depending on your end goal, having this can allow gradual consumption by different parties, allow old/weird platforms to not get the async version of things, and make it easier to identify those people who're still using the old stuff. But it also makes refactoring a tad bit harder, and in very large codebases people will still use the other interface while you're trying to kill it.

... or should I get rid of sync methods at all and go async only?

If you can. The performance benefits are significant and the pollution that occurs if people use the "wrong" thing can be fairly widespread.

What are the best practices of composing large types with both sync and async methods in a matter of file structure, code readability and reusability?

They vary significantly, because the technical challenges aren't the deciding factor usually. If your org has little appetite for giant refactoring work, you probably will need to take a more gradual approach. If you have a lot of consumers who can't effectively coordinate breaking changes, you probably will need to have some versioning approach. If you have a userbase that isn't very familiar with async/await, then you'll want to design things so it's easy to do the right thing. If you have a bunch of microservices or a SteamingPileOfGarbageRepository then it may be better to prototype out a parallel version that is async heavy...

Since those non-technical aspects are key to significant changes like adding async/await everywhere, consider those first and tailor the technical design to accommodate them.

3

I would go a pure async route because this is what you usually want and provide sync methods via extensions since they are only wrappers/decorators.

I would not use #regions for this purpose. Too much work and you have to provide all sync wrappers right away to have a complete API. Using partials has the same disadvantage.

But, I you have large types then having everything only async is much easier to maintain so I wouldn't even care about sync APIs because anyone needing them, can very quickly write sync extensions on their own. Providing sync APIs might just be a huge waste of time as there are very few scenarios when one would require them.

1

Suffix is all you need

If you've chosen to expose both async and non-async methods, the way to distinguish between them is with the async suffix. For example, Find() and FindAsync() provides sufficient distinction. And you're already doing that.

There is no reason to put them in regions or separate interfaces. Indeed, Find and FindAsync probably have much have more in common than they don't, so it makes sense to have them adjacent in intellisense (which is accomplished by using a suffix instead of a prefix) and in close proximity in the code base (which means no #region or partial classes are needed) as they are likely to share private utility functions and other dependencies and really belong together logically. The sync/async aspect of their execution is just an implementation detail. I would avoid organizing the code around that. There are probably better ways you could use regions or interfaces for this problem, e.g. to separate types of repositories by area of business.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.