Skip to content
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

implemented rick & morty task #10

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

knyazkyylubomir
Copy link

No description provided.

@Component
public class RickAndMortyClientImpl implements RickAndMortyClient {
private static final String BASE_URL = "https://rickandmortyapi.com/api/character/%s";
private int characterSize;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like idea to store data in bean class variable

Spring Data JPA provides a built-in count() method for each repository interface. You can use this method to count the number of rows in a table with characters

Comment on lines 50 to 53
@Override
public int getCharactersSize() {
return characterSize;
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be fetched from your DB

Comment on lines 23 to 27
int charactersSize = rickAndMortyClient.getCharactersSize();
long characterId
= new Random().nextLong(charactersSize - FIRST_CHARACTER_INDEX + ONE)
+ FIRST_CHARACTER_INDEX;
return characterRepository.findCharacterByExternalId(characterId)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd not rely on consistency of external ids
what if they have UUID or predefined step or some missing ids due to deletion of entries?
Please use reliable way to get random entry from your DB

Copy link

@Ivan95kos Ivan95kos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, with minor comments

@RequiredArgsConstructor
@Component
public class RickAndMortyClientImpl implements RickAndMortyClient {
private static final String BASE_URL = "https://rickandmortyapi.com/api/character/%s";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be stored in your app properties, and then be injected here via @value

characterRepository.saveAll(entityList);
saveNextCharacters(page, responseDto, httpClient);
} catch (IOException | InterruptedException e) {
throw new RuntimeException("Error occurred during receiving response", e);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider creating your custom exception

List<Character> entityList = characterMapper.toEntityList(responseDto.getResults());
characterRepository.saveAll(entityList);
} catch (IOException | InterruptedException e) {
throw new RuntimeException("Error occurred during receiving response", e);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants