Sobes.tech
Back to questions
Middle+
2

К вам на Code Review попала задача коллеги Какие проблемы в коде вы видите? ```csharp namespace SomeProject.Data { public class UserRepository { public Task SaveUser(User user, Dictionary<int,string> shards) { var shardIdx = user.Id % 3; var shardAddress = shards[shardIdx]; using(var shardService = new ShardService(shardAddress)) { return shardService.SaveUserAsync(user); } } } } ```

Companies where asked
Core outstaff

Answer from AI

sobes.tech AI

В представленном коде можно выделить несколько проблем:

  1. Неправильное использование using с асинхронным методом

    • Метод SaveUser возвращает Task, но внутри используется using для shardService, который, вероятно, реализует IDisposable. При этом shardService.SaveUserAsync(user) возвращает задачу, которая может выполняться после выхода из блока using, что приведёт к попытке использовать уже освобождённый объект.
  2. Отсутствие асинхронного модификатора и await

    • Метод должен быть объявлен как async Task, и вызов SaveUserAsync нужно await-ить, чтобы корректно дождаться завершения операции перед освобождением ресурсов.
  3. Жёстко заданное число шардов

    • В коде жёстко задано число 3 при вычислении shardIdx. Лучше использовать размер словаря shards.Count для гибкости.
  4. Отсутствие обработки ошибок

    • Нет обработки возможных исключений, например, если shardIdx отсутствует в словаре или если SaveUserAsync выбросит исключение.

Исправленный пример:

public async Task SaveUser(User user, Dictionary<int, string> shards)
{
    var shardIdx = user.Id % shards.Count;

    if (!shards.TryGetValue(shardIdx, out var shardAddress))
        throw new KeyNotFoundException($"Shard index {shardIdx} not found.");

    using(var shardService = new ShardService(shardAddress))
    {
        await shardService.SaveUserAsync(user);
    }
}

Таким образом, метод становится асинхронным, корректно ожидает завершения операции и безопасно освобождает ресурсы.