Постоянно возвращать Task или при необходимости использовать Task.Run()?

Question

Есть вот такой класс из слоя бизнес-логики:

public class ProductService
{
    private readonly IGenericRepository<Product> _productRepository;

    public ProductService(IGenericRepository<Product> productRepository)
    {
        _productRepository = productRepository;
    }

    public Task Update(int id, ProductModel model)
    {
        return Task.Run(() =>
        {
            if (model == null)
            {
                throw new ArgumentNullException(nameof(model));
            }

            var product = _productRepository.GetById(id);
            if (product == null)
            {
                throw new ArgumentException(nameof(id));
            }
            // еще код... //

            _productRepository.Update(product);
        });
    }
}

Я поймал себя на том, что у меня все методы, обращающиеся к базе данных (даже если они обращаются через репозитории), возвращают Task, так как знаю, что выше у меня Web API контроллер, action которого я хочу вызывать асинхронно.

Я думаю, что я делаю неправильно, потому что, проектируя класс ProductService я не должен заботиться о клиентском коде. Сейчас да, у меня клиентом является самый верхний слой (Web API), но так ведь может быть не всегда и для другого клиента возможно тип Task метода Update может быть излишним. Я прав?

Согласно своим рассуждениям я переписал методProductService.Update() и класс-клиент:

public void Update(int id, ProductModel model)
{
    if (model == null)
    {
        throw new ArgumentNullException(nameof(model));
    }

    var product = _productRepository.GetById(id);
    if (product == null)
    {
        throw new ArgumentException(nameof(id));
    }
    // еще код... //

    _productRepository.Update(product);
}


[RoutePrefix("api/products")]
public class ProductsController : ApiController
{
    private readonly IProductService _productService;

    public ProductsController(IProductService productService)
    {
        _productService = productService;
    }

    [HttpPut]
    [Route("{id:int}")]
    public async Task<IHttpActionResult> Put([FromBody] productModel)
    {
        if (!ModelState.IsValid)
        {
            return BadRequest(ModelState);
        }

        var productId = (int)RequestContext.RouteData.Values["id"];

        await Task.Run(() =>
        {
            _productService.Update(productId, productModel);
        });

        return Ok();
    }

score 8 · Accepted Answer · answered Jul 25 '17 at 12:32

8

В том что вы делаете, нет ни малейшего смысла - методы контроллеров уже исполняются в пуле потоков, и выносить их снова в пул потоков через Task.Run - бесполезно.

Если вам хочется сделать полностью асинхронный код - надо отказаться от Task.Run в пользу действительно асинхронных способов. Так, если вы используете EntityFramework, то вытащить запись из БД можно через FirstOrDefaultAsync, а сохранить изменения - через SaveChangesAsync

answered Jul 25 '17 at 12:32

Pavel Mayorov

58,537

Вы хотите сказать, что при запросе клиента к ProductsControlers.Put() автоматически создается отдельная нить ProductsControlers.Put() -> ProductService.Update() -> GenericRepoisoty<Product>.Update()? – Adam Shakhabov Jul 25 '17 at 12:44
1

@adamshakhabov такая нить создавалась бы, если бы вы не делали await Task.Run. Сейчас у вас используются две разные нити для той же цели. – Pavel Mayorov Jul 25 '17 at 12:47
Ох не зря я засомневался, что у меня тут какая-то муть). Значить, оставляю второй вариант ProductService.Update(), убираю async/await/Task.Run() из ProductsController.Put() и каждый запрос клиента на обновление сущности Product будет выполняться асинхронно, так? – Adam Shakhabov Jul 25 '17 at 12:53
1

@adamshakhabov вам надо в любом случае убрать все Task.Run, у вас оба варианта неправильные. – Pavel Mayorov Jul 25 '17 at 12:56
1

@adamshakhabov никаких Task.Run в коде быть вообще не должно если только вы намеренно не параллелите свой код. – Pavel Mayorov Jul 25 '17 at 12:57

score 4 · Answer 2 · answered Jul 25 '17 at 12:56

Я думаю, что я делаю неправильно, потому что, проектируя класс ProductService я не должен заботиться о клиентском коде.

Совершенно верно. Предоставьте возможность клиенту самому решать, когда ему запускать метод синхронно, а когда -- асинхронно. Создание асинхронных оберток для синхронных методов мы уже обсуждали.

Постоянно возвращать Task или при необходимости использовать Task.Run()?

2 Answers2