-
Notifications
You must be signed in to change notification settings - Fork 4
Fr] add the locale command to the cli. #24 #57
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
base: main
Are you sure you want to change the base?
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
| ) | ||
| public class PhoneBookCLI implements CliCommand { | ||
|
|
||
| private final SettingsServiceInterface settingsService; |
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.
The issue identified by the PMD linter suggests that the settingsService field may not need to be an instance variable (a class-level field) and could instead be a local variable. This typically occurs when the variable is not used outside of the constructor or the method where it is defined.
In this case, since settingsService is only being assigned in the constructor and not used elsewhere in the class, it can be made a local variable within the constructor.
Here's the code suggestion to fix the issue:
| private final SettingsServiceInterface settingsService; | |
| SettingsServiceInterface settingsService = settingsService; |
However, since the suggestion above does not make sense as it stands, the correct approach would be to eliminate the field altogether and use a local variable in the constructor if the class does not require the field to be accessible outside the constructor. If you want to maintain the functionality of using settingsService across methods, then the current structure is appropriate.
If settingsService is indeed used elsewhere in the class, then the PMD warning may not be applicable, and it could be suppressed or ignored.
If we need to keep it as a field and suppress the warning, we can use the following:
@SuppressWarnings("PMD.FieldNamingConventions")
private final SettingsServiceInterface settingsService;This would suppress the warning if you intend to keep the instance variable.
This comment was generated by an experimental AI tool.
| } | ||
|
|
||
| @Override | ||
| public void saveSingleSetting(String key, String value) throws IOException { |
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.
ℹ️ Codacy found a minor Code Style issue: Document empty method body
The issue identified by the PMD linter is that the method saveSingleSetting has an empty body, which is not documented. According to best practices in coding style, it's important to provide documentation for methods, especially when their implementation is not present, to clarify their intended purpose.
To fix this issue, you can add a comment documenting the method's purpose. Here’s the suggested change:
// This method saves a single setting identified by the key with the provided value.
public void saveSingleSetting(String key, String value) throws IOException {This comment was generated by an experimental AI tool.
| public class ForSettingsRepository implements SettingsRepository { | ||
|
|
||
| @Override | ||
| public void save(Map<String, String> settings) throws IOException { |
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.
ℹ️ Codacy found a minor Code Style issue: Document empty method body
The PMD linter is indicating that the method save(Map<String, String> settings) has an empty body, which is generally considered poor practice because it lacks documentation. It is important to document the purpose of the method, even if it currently does not contain any implementation. This helps other developers understand the intended functionality and context of the method.
To fix this issue, you can add a comment inside the method body to indicate that the method is intentionally left empty, which serves as documentation for future reference.
Here's the suggested code change:
| public void save(Map<String, String> settings) throws IOException { | |
| public void save(Map<String, String> settings) throws IOException { // Intentionally left empty |
This comment was generated by an experimental AI tool.
| contacts.add(contactDto); | ||
| saveAll(contacts); | ||
| } | ||
| default List<ContactDto> findByKeyword(String keyword) throws IOException { |
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.
ℹ️ Codacy found a minor Code Style issue: 'METHOD_DEF' should be separated from previous line.
The issue reported by the Checkstyle linter indicates that there should be a blank line separating the method definition findByKeyword from the previous method (save). This is a common code style guideline that helps improve readability by visually separating method definitions.
To fix this issue, you can add a blank line before the findByKeyword method definition. Here’s the single line change to resolve the issue:
<code suggestion>
default List<ContactDto> findByKeyword(String keyword) throws IOException {This comment was generated by an experimental AI tool.
| public interface ContactDtoRepository { | ||
| List<ContactDto> findAll(); | ||
| ObjectMapper objectMapper = new ObjectMapper().enable(SerializationFeature.INDENT_OUTPUT); | ||
| default List<ContactDto> findAll() throws IOException { |
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.
ℹ️ Codacy found a minor Code Style issue: 'METHOD_DEF' should be separated from previous line.
The issue reported by the Checkstyle linter indicates that there should be a blank line separating the method definition from the previous line. This is a common coding style guideline that improves readability by providing visual separation between different methods or sections of code.
To fix the issue, you can simply add a blank line before the findAll method definition. Here is the suggested change:
| default List<ContactDto> findAll() throws IOException { | |
| default List<ContactDto> findAll() throws IOException { |
This change maintains the existing functionality while adhering to the coding style guidelines.
This comment was generated by an experimental AI tool.
dRaider-bitrek
left a comment
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.
- Потрібно створити окремий клас public class SetLocale implements CliCommand{}
- Зараз команда не міняє локаль а завжди встановлює "uk"
| System.out.println("Locale set to Ukrainian."); | ||
| return 0; | ||
| }) | ||
| .execute(args); |
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.
Це потрібно робити в окремому класі наприклад:
@ Command(
name = "locale",
aliases = {"-l", "--locale"},
mixinStandardHelpOptions = true,
description = "")
public class SetLocale implements CliCommand{}
А саму локаль обробити за допомогою анотаціії:
@ Parameters(index = "0", description = "Фраза для пошуку", arity = "1")
private String newlocale;
тоді по команді> --locale "ua"
ми отримаємо підкапотом newlocale="ua;
Add Locale