Сообщение Re: Codereview - подскажите что в методе можно улучшить? от 16.03.2023 19:04
Изменено 17.03.2023 12:03 VladD2
Re: Codereview - подскажите что в методе можно улучшить?
Здравствуйте, Glestwid, Вы писали:
G>Вот, на интервью скинули такой метод на ревью. Что в нем можно улучшить и(или) ускорить, помимо добавления логирования ошибок ?
G> public async Task<string> ToHash(CreateHashCommand request, CancellationToken cancellationToken)
G> {
G> try
G> {
G> if (string.IsNullOrWhiteSpace(request.Url))
G> throw new InvalidEnumArgumentException();
G> var urlInDb = new Url()
G> {
G> Created = DateTime.UtcNow,
G> CreatedBy = "",
G> OriginalUrl = request.Url
G> };
G> await _context.Urls.AddAsync(urlInDb, cancellationToken);
G> await _context.SaveChangesAsync(cancellationToken);
G> var urlHash = _hashgen.EncodeLong(urlInDb.Id);
G> return urlHash;
G> }
G> catch (Exception ex)
G> {
G> throw;
G> }
G> }
1. Разнести все по слоям — хоть и спорно, но как минимум 2 слоя напрашивается. Соответственно как минимум отдельные методы по созданию сущности и сохранению
2. Работа с ошибками — зачем пользователю АПИ видеть, что там именно БД накрылась медным тазом? Должны быть скорее всего иные ошибки.
3. Трейсинг.
4. Очень странная ошибка на пустой урл. Я бы вообще валидатор прикрутил бы с нормальной проверкой на то, что это именно урл, а не три буквы.
5. Непонятно, что будет если урл уже в базе? В чем бизнес-цель метода?
G>Вот, на интервью скинули такой метод на ревью. Что в нем можно улучшить и(или) ускорить, помимо добавления логирования ошибок ?
G> public async Task<string> ToHash(CreateHashCommand request, CancellationToken cancellationToken)
G> {
G> try
G> {
G> if (string.IsNullOrWhiteSpace(request.Url))
G> throw new InvalidEnumArgumentException();
G> var urlInDb = new Url()
G> {
G> Created = DateTime.UtcNow,
G> CreatedBy = "",
G> OriginalUrl = request.Url
G> };
G> await _context.Urls.AddAsync(urlInDb, cancellationToken);
G> await _context.SaveChangesAsync(cancellationToken);
G> var urlHash = _hashgen.EncodeLong(urlInDb.Id);
G> return urlHash;
G> }
G> catch (Exception ex)
G> {
G> throw;
G> }
G> }
1. Разнести все по слоям — хоть и спорно, но как минимум 2 слоя напрашивается. Соответственно как минимум отдельные методы по созданию сущности и сохранению
2. Работа с ошибками — зачем пользователю АПИ видеть, что там именно БД накрылась медным тазом? Должны быть скорее всего иные ошибки.
3. Трейсинг.
4. Очень странная ошибка на пустой урл. Я бы вообще валидатор прикрутил бы с нормальной проверкой на то, что это именно урл, а не три буквы.
5. Непонятно, что будет если урл уже в базе? В чем бизнес-цель метода?
Re: Codereview - подскажите что в методе можно улучшить?
Здравствуйте, Glestwid, Вы писали:
G>Вот, на интервью скинули такой метод на ревью. Что в нем можно улучшить и(или) ускорить, помимо добавления логирования ошибок ?
1. Разнести все по слоям — хоть и спорно, но как минимум 2 слоя напрашивается. Соответственно как минимум отдельные методы по созданию сущности и сохранению
2. Работа с ошибками — зачем пользователю АПИ видеть, что там именно БД накрылась медным тазом? Должны быть скорее всего иные ошибки.
3. Трейсинг.
4. Очень странная ошибка на пустой урл. Я бы вообще валидатор прикрутил бы с нормальной проверкой на то, что это именно урл, а не три буквы.
5. Непонятно, что будет если урл уже в базе? В чем бизнес-цель метода?
G>Вот, на интервью скинули такой метод на ревью. Что в нем можно улучшить и(или) ускорить, помимо добавления логирования ошибок ?
1. Разнести все по слоям — хоть и спорно, но как минимум 2 слоя напрашивается. Соответственно как минимум отдельные методы по созданию сущности и сохранению
2. Работа с ошибками — зачем пользователю АПИ видеть, что там именно БД накрылась медным тазом? Должны быть скорее всего иные ошибки.
3. Трейсинг.
4. Очень странная ошибка на пустой урл. Я бы вообще валидатор прикрутил бы с нормальной проверкой на то, что это именно урл, а не три буквы.
5. Непонятно, что будет если урл уже в базе? В чем бизнес-цель метода?