Информация об изменениях

Сообщение Re: Codereview - подскажите что в методе можно улучшить? от 16.03.2023 19:32

Изменено 16.03.2023 19:53 bnk

Re: Codereview - подскажите что в методе можно улучшить?
Здравствуйте, 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;
    }
}


Я в принципе согласен
Re: Codereview - подскажите что в методе можно улучшить?
Здравствуйте, 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/