app ready #46
app ready #46
Conversation
| implementationPackage = "<PACKAGE_NAME>.impl" | ||
| ) | ||
| public class MapperConfig { | ||
|
|
There was a problem hiding this comment.
I'm not entirely sure what you meant here and in the other similar reviews.
| @RequiredArgsConstructor | ||
| public class AppStartupRunner implements ApplicationRunner { | ||
| private final CharacterRepository repository; | ||
|
|
|
|
||
| @Override | ||
| public void run(ApplicationArguments args) throws Exception { | ||
| if (characterClient.getDataByPage(1).getInfo().getCount() != repository.count()) { |
There was a problem hiding this comment.
is it up-to-date data check? consider extracting it to self-explanatory named method
| if (characterClient.getDataByPage(1).getInfo().getCount() != repository.count()) { | |
| if (characterClient.getDataByPage(1).getInfo().getCount() != repository.count()) { |
| if (characterClient.getDataByPage(1).getInfo().getCount() != repository.count()) { | ||
| List<Character> allCharacters = characterClient.getAllCharacters(); | ||
| for (Character character : allCharacters) { | ||
| if (!repository.findByExternalId(character.getExternalId()).isPresent()) { | ||
| repository.save(character); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
all this logic can be separated to the DataLoader service (too low abstraction on high-level ApplicationRunner)
| } | ||
| } | ||
| } | ||
| System.out.println(characterClient.getDataByPage(1)); |
There was a problem hiding this comment.
what's purpose of this log? better do debug, also we have @Slf4j logger embed in Lombok
| System.out.println(characterClient.getDataByPage(1)); | |
| log.debug(characterClient.getDataByPage(1)); |
| private static final String BASE_URL = "https://rickandmortyapi.com/api/character/?page=%s"; | ||
|
|
||
| private final ObjectMapper objectMapper; | ||
|
|
| } | ||
|
|
||
| public CharacterResponseDataDto getDataByPage(int page) { | ||
| HttpClient client = HttpClient.newHttpClient(); |
There was a problem hiding this comment.
as we are using Spring, consider creating @Bean public HttpClient httpClient()... bean in your Spring Configuration, so we could autowire it to the private final class-level field
|
|
||
| @Override | ||
| public CharacterDto getRandomCharacter() { | ||
| Random random = new Random(); |
There was a problem hiding this comment.
should be class-level field, injected by Spring
| @RequiredArgsConstructor | ||
| public class CharacterServiceImpl implements CharacterService { | ||
| private final CharacterRepository characterRepository; | ||
|
|
| @@ -19,8 +19,45 @@ | |||
| <maven.checkstyle.plugin.configLocation> | |||
|
I didn't quite understand some problems you pointed out. The ones with the -. Would be great if you clarified. |
No description provided.