code refactoring #22
code refactoring #22
Conversation
| @@ -23,6 +23,109 @@ | |||
| @RequestMapping("api/import") | |||
| public class ImportCSV { | |||
|
|
|||
| class ParsedEntity { | |||
There was a problem hiding this comment.
great idea to create an inner class to ease parsing :)
| private int pos; | ||
|
|
||
| /*** | ||
| * contructor |
There was a problem hiding this comment.
I guess this comments were not finished because of time reasons...just want to comment that a more meaning full text would help why you added this class
| * @param pet | ||
| * @return | ||
| */ | ||
| public boolean deregisterPetFromOwner(Pet pet) { |
There was a problem hiding this comment.
I like that you kept the indentations small throughout the class :) except maybe in this class, the for-loop and three if-clauses could be eased a bit
| HttpHeaders headers = new HttpHeaders(); | ||
| if (matchingOwners.size() == 0) { | ||
| headers.add("errors", "Owner not found"); | ||
| return new ResponseEntity<List<Pet>>(headers, HttpStatus.BAD_REQUEST); |
There was a problem hiding this comment.
Nice that you return meaningful error messages
| @@ -33,106 +136,62 @@ | |||
| produces = "application/json") | |||
| public ResponseEntity<List<Pet>> importPets(@RequestBody String csv) { | |||
|
|
|||
| int i = 0; | |||
| int pos = 0; | |||
There was a problem hiding this comment.
this method is maybe still a bit long...maybe you could extract some methods to create e.g., the importPets method only handles the http request, and then you have a new methods that orchestrates the parsing and then you have many small methods that to the parsing of the various fields and updating the database
refactoring from group 4