Conversation
| { | ||
| return Ok(_tripManager | ||
| .FindTrips(requestRoute.DepartureCityName, requestRoute.DestinationCityName, requestRoute.DepartDate)); | ||
| throw new NotImplementedException(); |
There was a problem hiding this comment.
лучше возвращай 501 ошибку "NotImplemented". иначе апи возвращает 500 и клиент ничего не понимает.
https://ru.wikipedia.org/wiki/%D0%A1%D0%BF%D0%B8%D1%81%D0%BE%D0%BA_%D0%BA%D0%BE%D0%B4%D0%BE%D0%B2_%D1%81%D0%BE%D1%81%D1%82%D0%BE%D1%8F%D0%BD%D0%B8%D1%8F_HTTP
вот на будущее
| _tripManager.BookTrip(tripId); | ||
| _accountManager.AddTripToAccount(tripId, userId); | ||
| return Ok(); | ||
| throw new NotImplementedException(); |
There was a problem hiding this comment.
аналогично тому, что выше
| public class BankCardModel | ||
| { | ||
| [Required] | ||
| [JsonProperty("cardNumber")] |
There was a problem hiding this comment.
десериалайзер ньютонсофта достаточно умный, чтобы понять, что это одни и те же поля, просто с разным регистром
There was a problem hiding this comment.
так лучше, эта хуйня позволит мне использовать свои названия и не зависеть от того как оно сериализуется или десериализуется. И в случае изменения, названия поля ничего не сломается
There was a problem hiding this comment.
Ну, пока что это просто лишняя строчка кода.
Еще я не знаю как себя будет вести newtonsoft, если тебе в данном случае придет такое поле с верхним регистром. Л
учше протести это: если поле нормально будет десериализовываться, то оставь, если нет, то лучше убрать и добавлять такие пропсы по мере необходимости
| [Required] | ||
| [JsonProperty("cardNumber")] | ||
| [RegularExpression("^[0-9]{15}(?:[0-9]{1})?$")] | ||
| public string CardNumber { get; set; } |
There was a problem hiding this comment.
лучше избавиться от публичных сеттеров и создать конструктор. так у тебя объекты будут оставаться немутабельными
There was a problem hiding this comment.
в моделях которые не используются в логике это норм, хз. Это делается для десериалайзера. Можно и через конструктор, но если будет какая-то сложная модель, то там уже сложнее. И атрибуты можно классные юзать
There was a problem hiding this comment.
Если у тебя нет каких-то наследований, то все должно быть хорошо
T-PortBackend/TPortApi/Startup.cs
Outdated
| @@ -60,14 +59,8 @@ public void ConfigureServices(IServiceCollection services) | |||
| var smsManager = new SmsManager(); | |||
There was a problem hiding this comment.
подобные зависимостти не обязательно создавать вручную. DI автоматически создает объект, если знает все параметры конструктора. Достаточно просто указать реализацию. Например:
services.AddSingleton<Abstraction, Implementation>();
| _totpManager.ValidateToken(loginConfirmationRequest.Phone, int.Parse(loginConfirmationRequest.Code)); | ||
| _totpManager.ValidateToken(loginConfirmationRequest.Phone, loginConfirmationRequest.Code); | ||
| var account = _accountManager.LoadAccount(loginConfirmationRequest.Phone); | ||
| var credentials = new Credentials(Request.Headers["User-Agent"], loginConfirmationRequest.Phone); |
There was a problem hiding this comment.
лучше добавить проверку хэдера на null, и выбрасывать BadRequest
There was a problem hiding this comment.
там внутри Credentials проверяется на null
| var result = accountController.Login(loginRequest); | ||
|
|
||
| // Assert | ||
| Assert.NotNull(result as OkResult); |
There was a problem hiding this comment.
ответ никогда не будет null
|
|
||
| public void SendMessage(string phoneNumber, string message) | ||
| { | ||
| if (phoneNumber == null) throw new ArgumentNullException(nameof(phoneNumber)); |
There was a problem hiding this comment.
phoneNumber ?? throw new ArgumentNullException(nameof(phoneNumber));
| { | ||
| if (phoneNumber == null) throw new ArgumentNullException(nameof(phoneNumber)); | ||
|
|
||
| if (message == null) throw new ArgumentNullException(nameof(message)); |
3 participants
No description provided.