Yuhua's PR #3
Yuhua's PR #3
Conversation
| [HttpGet("{id}")] | ||
| public async Task<ActionResult<ParkingLotDto>> GetParkingLost(string id) | ||
| { | ||
| return StatusCode(StatusCodes.Status200OK, await parkingService.GetParkLot(id)); |
There was a problem hiding this comment.
WangKe: should return 404 not found for non-existing ids
| context.Result = new BadRequestResult(); | ||
| context.ExceptionHandled = true; | ||
| } | ||
| if (context.Exception is FormatException) |
There was a problem hiding this comment.
WangKe: Good use of filter for format exception handling
| [HttpGet] | ||
| public async Task<ActionResult> GetParkingLosts(int pageIndex = 0) | ||
| { | ||
| if (pageIndex < 0) |
There was a problem hiding this comment.
WangKe: good consideration for the pageIndex invalidation
|
|
||
| if (parkingLotEntity == null) | ||
| { | ||
| throw new DuplicateNameException(); |
There was a problem hiding this comment.
WangKe: good consideration for the name duplication exception
|
|
||
| namespace ParkingLotApi.Exceptions | ||
| { | ||
| public class DuplicateNameException : Exception |
There was a problem hiding this comment.
well: nice to use customized exception to present business issue
| { | ||
| int IOrderedFilter.Order => int.MaxValue - 10; | ||
|
|
||
| public void OnActionExecuted(ActionExecutedContext context) |
There was a problem hiding this comment.
well: nice to handle more than one exception
| @@ -0,0 +1,9 @@ | |||
| namespace ParkingLotApi.Dtos | |||
| { | |||
| public class ParkingLotRequestDto | |||
| using ParkingLotApi.Dtos; | ||
| using ParkingLotApi.Models; | ||
|
|
||
| namespace ParkingLotApi.Utils |
There was a problem hiding this comment.
suggestion: the class can be moved to Services, since it's only use in service
|
|
||
| public async Task<List<ParkingLotDto>?> GetParkLots(int pageIndex) | ||
| { | ||
| int skipItems = (pageIndex - 1) * 15; |
There was a problem hiding this comment.
suggestion: 15 can be extract as a constant
No description provided.