-
Notifications
You must be signed in to change notification settings - Fork 256
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Solution #202
base: main
Are you sure you want to change the base?
Solution #202
Conversation
Mapper doesn't work with < sourceDirectories > src < / sourceDirectories >) in pom.xml |
Not sure about solution, give some edvice ;) |
import org.springframework.web.bind.annotation.RequestMapping; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
||
@Tag(name = "Rick&MortyApi", description = "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty description is not very good
@Operation(summary = "Add random hero", description = """ | ||
The request randomly generates a wiki about one character in | ||
the universe the animated series Rick & Morty.""") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Does this endpoint add a random hero or return a random one?
@GetMapping("/find") | ||
List<InternalHeroDto> heroesByName(String name) { | ||
return rickAndMortyService.findByName(name); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- You don't use a @PathVariable or @RequestParam, where do you get parameter "name" from?)
@GetMapping("/find") | |
List<InternalHeroDto> heroesByName(String name) { | |
return rickAndMortyService.findByName(name); | |
} | |
@GetMapping | |
List<InternalHeroDto> getCharacterByName(@RequestParam String name) { | |
return rickAndMortyService.findByName(name); | |
} |
@Operation(summary = "Add random hero", description = """ | ||
The request randomly generates a wiki about one character in | ||
the universe the animated series Rick & Morty.""") | ||
@GetMapping("/get") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GetMapping("/get") | |
@GetMapping("/random") |
public InternalHeroDto rickAndMorty() { | ||
return rickAndMortyService.getRandomCharacter(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You use 2 different names: hero and character for the same thing through all your project. Please, stick to one of them, because it can be confusing
|
||
@Override | ||
public ExternalHeroDto getRandomHero(int randomNumber) { | ||
HttpClient httpClient = HttpClient.newHttpClient(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to the constants
return objectMapper | ||
.readValue(response.body(), ExternalHeroDto.class); | ||
} catch (IOException | InterruptedException e) { | ||
throw new RuntimeException(e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add an informative error message
return objectMapper | ||
.readValue(response.body(), ExternalHeroResponseDto.class).result(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return objectMapper | |
.readValue(response.body(), ExternalHeroResponseDto.class).result(); | |
return objectMapper | |
.readValue(response.body(), ExternalHeroResponseDto.class) | |
.result(); |
ExternalHeroDto randomHero = rickAndMortyClient | ||
.getRandomHero(new Random().nextInt(API_COUNT_HEROES)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move Random
to the constants
return rickAndMortyClient | ||
.findHeroesByName(name).stream() | ||
.map(e -> { | ||
Hero entity = heroMapper.toEntity(e); | ||
return heroMapper.toDto(heroRepository.save(entity)); | ||
}) | ||
.toList(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need always call rickAndMortyClient.findHeroesByName and perform a HTTP call + save the same values in DB (if name is repeated), instead you can load this values from DB
No description provided.