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

payer plan period criteria support for circe-be #11

Merged
merged 3 commits into from
Jan 3, 2018

Conversation

raomane
Copy link
Contributor

@raomane raomane commented Oct 4, 2017

Related to OHDSI/CommonDataModel#120
If/Once the proposal is accepted
@chrisknoll
OHDSI/WebAPI#217

public NumericRange periodLength;

@JsonProperty("AgeAtStart")
public NumericRange ageAtstart;
Copy link

Choose a reason for hiding this comment

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

@raomane, @gowthamrao - hello, do you mean "ageAtStart"?

Copy link
Member

Choose a reason for hiding this comment

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

agree

ArrayList<String> whereClauses = new ArrayList<>();

//first
if (criteria.first != null && criteria.first == true)
Copy link

Choose a reason for hiding this comment

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

I think ... && criteria.first is enough

Copy link
Member

Choose a reason for hiding this comment

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

i think this is inline with other patterns.
e.g.

if (criteria.first != null && criteria.first == true)

if (criteria.first != null && criteria.first == true)

if (criteria.first != null && criteria.first == true)

etc

Copy link
Collaborator

@chrisknoll chrisknoll left a comment

Choose a reason for hiding this comment

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

Local compile works, although we do not have a CDM v5.3 implementation to validate with.

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.

4 participants