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

commit #824

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

Volodymyr-Mykychak
Copy link

No description provided.

Copy link

@kozhukhovsky kozhukhovsky left a comment

Choose a reason for hiding this comment

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

Very good job for first try!

contentFromFile.stream()
.map(PARSER_FILE::parseLine)
.forEach(transaction -> operationStrategyMap
.get(Operation.checkTypeOperation(transaction.getOperation()))

Choose a reason for hiding this comment

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

I would create an OperationStrategy class, which will just be responsible for the logic of obtaining a handler for the desired operation.

@@ -0,0 +1,5 @@
package core.basesyntax.service;

public interface FruitService {

Choose a reason for hiding this comment

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

Suggested change
public interface FruitService {
public interface ReportService {

import java.util.Map;

public class FruitServiceImpl implements FruitService {

Choose a reason for hiding this comment

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

Suggested change

public class FruitServiceImpl implements FruitService {

@Override
public String getReport() {

Choose a reason for hiding this comment

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

Potentially, it's better not to use internal storage, so we're hardwired to the class and it can't be used for anything else. Processing in the same way, only take the map not from storage, but pass it as a parameter to the method, but when calling, pass it from storage.

import java.util.List;

public class ReaderServiceImpl implements ReaderService {

Choose a reason for hiding this comment

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

Suggested change

} catch (IOException e) {
throw new RuntimeException("Can't read from file " + filePath);
}
return content;

Choose a reason for hiding this comment

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

Suggested change
return content;

import java.io.FileWriter;

public class WriterServiceImpl implements WriterService {

Choose a reason for hiding this comment

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

Suggested change

import core.basesyntax.dto.Transaction;

public class AddOperationHandler implements OperationHandler {

Choose a reason for hiding this comment

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

Suggested change

import core.basesyntax.dto.Transaction;

public class BalanceOperationHandler implements OperationHandler {

Choose a reason for hiding this comment

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

Suggested change

import core.basesyntax.dto.Transaction;

public class PurchaseOperationHandler implements OperationHandler {

Choose a reason for hiding this comment

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

Suggested change

@Volodymyr-Mykychak
Copy link
Author

please check the new commit

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.

None yet

2 participants