Skip to content

Support for additional TaskModel Survey element fields#485

Merged
dephillipsmichael merged 7 commits intoResearchStack:developfrom
dephillipsmichael:motivation
Oct 16, 2018
Merged

Support for additional TaskModel Survey element fields#485
dephillipsmichael merged 7 commits intoResearchStack:developfrom
dephillipsmichael:motivation

Conversation

@dephillipsmichael
Copy link
Contributor

No description provided.

public class ChoiceQuestionSurveyItem extends QuestionSurveyItem<Choice> {
/* Default constructor needed for serilization/deserialization of object */
ChoiceQuestionSurveyItem() {
public ChoiceQuestionSurveyItem() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to increase visibility of these constructors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, otherwise there are no constructors for it (only GSON)

//-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*-*
private QuestionStep step;
private StepResult<String> result;
protected QuestionStep step;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to increase visibility? Should we make the step and result fields final?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sub-classes should have access to these.

Copy link
Collaborator

@liujoshua liujoshua left a comment

Choose a reason for hiding this comment

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

LGTM, just some questions around visibility

@dephillipsmichael dephillipsmichael merged commit 010cd56 into ResearchStack:develop Oct 16, 2018
@dephillipsmichael dephillipsmichael deleted the motivation branch October 16, 2018 21:13
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.

2 participants