Pull Request de Adrián De Simone #135
Conversation
|
Buenas! Te dejo algunas preguntas y sugerencias:
Dejo también algunos comentarios en las líneas de código especificas. |
| /// </summary> | ||
| /// <param name="name">Provider name to check.</param> | ||
| /// <returns></returns> | ||
| private bool NombreUnico(string name) |
There was a problem hiding this comment.
Esta funcion es identica a la que esta en la clase StoreService. Se te ocurre alguna forma de refactorizar el codigo para evitar duplicacion?
| /// <param name="id">Provider id.</param> | ||
| /// <param name="value">Provider information.</param> | ||
| [HttpPut("{id}")] | ||
| public void Put(string id, [FromBody] ProviderDTO value) |
There was a problem hiding this comment.
Que pasa si el parámetro id es distinto al del id contenido dentro del parámetro value?
| } | ||
| catch (Exception ex) | ||
| { | ||
| logger.LogCritical(ex.StackTrace); |
There was a problem hiding this comment.
¿Por qué se usa critical? ¿Por qué el mensaje de la respuesta es que ya existe si esta en un catch general?
| /// <param name="value">Provider info.</param> | ||
| /// <returns></returns> | ||
| [HttpPost] | ||
| public ActionResult Post([FromBody] ProviderDTO value) |
There was a problem hiding this comment.
Cual es la diferencia entre usar como respuesta un ActionResult, ActionResult<ProviderDTO> y ProviderDTO? Cual te parece más adecuado?
Description
Actividad inicial realizada. Los test corrieron de forma satisfactoria