Skip to content

If else branching - #449

Open
vargaeszter wants to merge 5 commits into
eclipse-arrowhead:developmentfrom
vargaeszter:IfElseBranching
Open

If else branching#449
vargaeszter wants to merge 5 commits into
eclipse-arrowhead:developmentfrom
vargaeszter:IfElseBranching

Conversation

@vargaeszter

@vargaesztervargaeszter commented Nov 21, 2023

Copy link
Copy Markdown

No description provided.

borditamas
borditamas reviewed Nov 22, 2023
View reviewed changes
Comment threadchoreographer/src/main/java/eu/arrowhead/core/choreographer/service/StepResponseEvaluation.java Outdated
class StepResponseEvaluation {

private String getJsonValue(String path, String message) {
ObjectMapper objectMapper = new ObjectMapper();

@borditamasborditamasNov 22, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should @Autowire the object mapper and not instatatiate it every time when the method is called.

borditamas
borditamas reviewed Nov 22, 2023
View reviewed changes
Comment threadcore-common/src/main/java/eu/arrowhead/common/database/entity/ChoreographerStep.java Outdated
borditamas
borditamas reviewed Nov 22, 2023
View reviewed changes
Comment threadcore-common/src/main/java/eu/arrowhead/common/database/entity/ChoreographerStep.java Outdated
borditamas
borditamas reviewed Nov 22, 2023
View reviewed changes
Comment threadcore-common/src/main/java/eu/arrowhead/common/database/entity/ChoreographerStep.java Outdated
borditamas
borditamas reviewed Nov 22, 2023
View reviewed changes
Comment threadcore-common/src/main/java/eu/arrowhead/common/database/entity/ChoreographerStep.java Outdated
borditamas
borditamas requested changes Nov 22, 2023
View reviewed changes

@borditamasborditamas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See inline comments.

Also, unit tests will need to be updated/written later to cover this new feature and you need to become an Eclipse Contributor (wiki) for the offical release.

@borditamas
borditamas changed the base branch from master to developmentDecember 8, 2023 07:15
borditamas
borditamas requested changes Dec 8, 2023
View reviewed changes

@borditamasborditamas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There are still some suggestion, but looks good overall.

Comment threadchoreographer/src/main/java/eu/arrowhead/core/choreographer/service/ChoreographerService.java

//-------------------------------------------------------------------------------------------------
private ExecutorSelector executorSelector;

@borditamasborditamasDec 8, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add here:

@Autowired
private StepResponseEvaluation evaluator;

Comment threadchoreographer/src/main/java/eu/arrowhead/core/choreographer/service/ChoreographerService.java Outdated

boolean executable = true;

AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();

@borditamasborditamasDec 8, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Delete these stuffs:

AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext();
context.scan("eu.arrowhead.core.choreographer.servic");
StepResponseEvaluation evaluator = new StepResponseEvaluation(); //context.getBean(StepResponseEvaluation.class); // new StepResponseEvaluation();
'''

Comment threadchoreographer/src/main/java/eu/arrowhead/core/choreographer/service/StepResponseEvaluation.java
class StepResponseEvaluation {

private final Logger logger = LogManager.getLogger(StepResponseEvaluation.class);

@borditamasborditamasDec 8, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add:

@Autowired
private ObjectMapper mapper;

Comment threadchoreographer/src/main/java/eu/arrowhead/core/choreographer/service/StepResponseEvaluation.java Outdated
public Boolean stepOutputValue(String message, String path, String threshold) {
// if sth used is null throw exception
if (path == null || message == null || threshold == null) {
throw new NullPointerException("The arguments cannot be null");

@borditamasborditamasDec 8, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do not throw programatically throw NullPointerException. It should be thrown only in runtime when it is not intentional.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@borditamasborditamasborditamas requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vargaeszter@borditamas