Re: Codereview - подскажите что в методе можно улучшить?
От: bnk СССР http://unmanagedvisio.com/
Дата: 16.03.23 19:32
Оценка: 12 (3) +1
Здравствуйте, Glestwid, Вы писали:

G>Вот, на интервью скинули такой метод на ревью. Что в нем можно улучшить и(или) ускорить, помимо добавления логирования ошибок ?



Скорми ChatGPT:

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

Имя метода ToHash не является самым информативным. Рекомендуется использовать более понятные имена, такие как GenerateUrlHashAsync или CreateUrlHashAsync.

Вместо InvalidEnumArgumentException лучше использовать ArgumentException или создать собственный класс исключения, например InvalidUrlException, так как InvalidEnumArgumentException обычно используется для проверки аргументов типа перечисления (enum).

В качестве CreatedBy используется пустая строка. Если это поле не является обязательным, возможно, стоит использовать null или добавить код для получения имени пользователя или системного аккаунта.

Добавьте XML-комментарии к методу, чтобы объяснить его назначение и параметры.

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

Вот пример кода с учетом предложенных изменений:


/// <summary>
/// Generates a hash for the given URL, saves it to the database, and returns the hash.
/// </summary>
/// <param name="request">A CreateHashCommand containing the URL to hash.</param>
/// <param name="cancellationToken">A CancellationToken to observe while waiting for the task to complete.</param>
/// <returns>A Task that represents the asynchronous operation, containing the generated URL hash as a string.</returns>
public async Task<string> GenerateUrlHashAsync(CreateHashCommand request, CancellationToken cancellationToken)
{
    try
    {
        if (string.IsNullOrWhiteSpace(request.Url))
            throw new ArgumentException("Url cannot be null or empty.", nameof(request.Url));

        var urlInDb = new Url()
        {
            Created = DateTime.UtcNow,
            CreatedBy = null,
            OriginalUrl = request.Url
        };

        await _context.Urls.AddAsync(urlInDb, cancellationToken);
        await _context.SaveChangesAsync(cancellationToken);

        var urlHash = _hashgen.EncodeLong(urlInDb.Id);

        return urlHash;
    }
    catch (Exception ex)
    {
        // Add logging or error handling here.
        throw;
    }
}


Я в принципе согласен

Добавка насчет AddAsync — в данном контексте не имеет особого смысла (можно использовать Add), возможно дадут bonus point:
https://thesharperdev.com/ef-cores-addasync-v-add-method/

О еще нашел, вроде лежит на поверхности.
Какой смысл передавать весь request если из него используется только Url. Лишняя зависимость.
Не разумнее будет только Url и передать как параметр?
Отредактировано 17.03.2023 12:03 bnk . Предыдущая версия . Еще …
Отредактировано 16.03.2023 19:53 bnk . Предыдущая версия .
Отредактировано 16.03.2023 19:33 bnk . Предыдущая версия .
 
Подождите ...
Wait...
Пока на собственное сообщение не было ответов, его можно удалить.