The aqlparser from nedap

AQL validation error contains no processable line number

Contains extra information only in the message - it should also be in a variable so it can be actually used. Preferably for all errors, even validation errors, so it can be shown in for example an IDE extension.

Review feedback

My first round of feedback:

aqlparser/src/main/java/com/nedap/healthcare/aqlparser/exception/AQLUnsupportedFeatureException.java

Line 5 in 233735b

public AQLUnsupportedFeatureException() {}

This is unused.

aqlparser/src/main/java/com/nedap/healthcare/aqlparser/exception/AQLValidationException.java

Lines 5 to 7 in 233735b

 public AQLValidationException(Exception e) { 

 super(e); 

 }

This is unused.

aqlparser/src/main/java/com/nedap/healthcare/aqlparser/model/leaf/ArchetypeId.java

Lines 12 to 15 in 233735b

 @Override 

 public void validate() { 

 new ArchetypeHRID(getValue()); 

 }

This throws an IllegalArgumentException instead of an AQLValidationException as other validate methods do. Please catch the IllegalArgumentException and convert it to an AQLValidationException.

aqlparser/src/main/java/com/nedap/healthcare/aqlparser/model/leaf/IdentifiedExprOperand.java

Lines 65 to 67 in 233735b

 public ClassExprOperand getClassExprOperand() { 

 return getLookup().getClassExprOperand(getVariableName()); 

 }

This is unused. I'm not sure why.

aqlparser/src/main/java/com/nedap/healthcare/aqlparser/model/leaf/ObjectPath.java

Lines 32 to 46 in 233735b

 public Boolean isCompositionPath() { 

 String identifier = pathParts.get(0).getIdentifier().toLowerCase(); 

 //Check if first identifier equals 

 switch (identifier) { 

 case "language": 

 case "territory": 

 case "category": 

 case "context": 

 case "composer": 

 case "content": 

 return true; 

 default: 

 return false; 

 } 

 }

Some of these attributes are also attributes on other RM classes, e.g. language is also an attribute on OBSERVATION and DV_TEXT. Can't this be solved another way?

aqlparser/src/main/java/com/nedap/healthcare/aqlparser/model/leaf/Operator.java

Lines 6 to 7 in 233735b

 import java.time.LocalDateTime; 

 import java.time.OffsetDateTime;

These are unused.

aqlparser/src/main/java/com/nedap/healthcare/aqlparser/model/leaf/Operator.java

Lines 22 to 24 in 233735b

 public Operator(String type) { 

 this.type = OperatorType.get(type); 

 }

This is unused.

aqlparser/src/main/java/com/nedap/healthcare/aqlparser/model/Lookup.java

Lines 61 to 63 in 233735b

 public String getParameterKey(PrimitiveOperand primitiveOperand) { 

 return parameter.entrySet().stream().filter(primitiveOperand::equals).map(Map.Entry::getKey).findFirst().get(); 

 }

I don't think this works. I think you mean

filter(entry -> primitiveOperand.equals(entry.getValue()))

aqlparser/src/main/java/com/nedap/healthcare/aqlparser/model/NodeExpression.java

Line 149 in 233735b

protected void addChild(ParseTree parseTree) {

This is unused.

All protected methods in NodeExpression can be changed to private.

aqlparser/src/main/java/com/nedap/healthcare/aqlparser/parser/QOMParser.java

Lines 52 to 53 in 233735b

 final QOMParser visitor = new QOMParser(lookup); 

 QOMObject object = visitor.visit(parseTree);

can be changed to

QOMObject object = QOMParserUtil.parse(lookup, parseTree);

Then this class doesn't need to extend AQLBaseVisitor anymore and private Lookup lookup;, the constructor and public QOMObject visitChildren(RuleNode node) { ... } can be removed.

small issues

octal should go up to 7, not 8
our date format is more extended than the specification grammar. Probably keep as is (see text in spec!) but note on top of file
the IDENTIFIER starts with aA subcase is to prevent conflict with at-codes. looks like it's no longer needed in ANTLR4 because of the longest match gaining preference, and the at-code having priority over the IDENTIFIER, as it probably should be?

remove Nedap specifics from readme

tipped by @boeckers:
remove artifactory part (or clarify it's Nedap specific)
idem for the references to chiron.

AQL QOMparser throws exception on the first syntax error it finds instead of returning multiple

Instead of listing all syntax errors, it just throws an exception. Of course I can manually do that by just using the grammar - but shouldn't it just list all of them instead?

nedap / aqlparser Goto Github PK

aqlparser's People

Contributors

Stargazers

Watchers

aqlparser's Issues

AQL validation error contains no processable line number

Review feedback

small issues

remove Nedap specifics from readme

AQL QOMparser throws exception on the first syntax error it finds instead of returning multiple

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	@Override
	public void validate() {
	new ArchetypeHRID(getValue());
	}

	public ClassExprOperand getClassExprOperand() {
	return getLookup().getClassExprOperand(getVariableName());
	}

	public Boolean isCompositionPath() {
	String identifier = pathParts.get(0).getIdentifier().toLowerCase();
	//Check if first identifier equals
	switch (identifier) {
	case "language":
	case "territory":
	case "category":
	case "context":
	case "composer":
	case "content":
	return true;
	default:
	return false;
	}
	}

	import java.time.LocalDateTime;
	import java.time.OffsetDateTime;

	public Operator(String type) {
	this.type = OperatorType.get(type);
	}

	public String getParameterKey(PrimitiveOperand primitiveOperand) {
	return parameter.entrySet().stream().filter(primitiveOperand::equals).map(Map.Entry::getKey).findFirst().get();
	}

	final QOMParser visitor = new QOMParser(lookup);
	QOMObject object = visitor.visit(parseTree);