-
Notifications
You must be signed in to change notification settings - Fork 5
Add interface for complex LSInput #6
base: master
Are you sure you want to change the base?
Conversation
physikerwelt
commented
May 1, 2018
- Limit the max depth to avoid being trapped in cycles
* Limit the max depth to avoid being trapped in cycles
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks a lot for the PR @physikerwelt. Albeit, it's a rather simple cycle check (I think it requires a proper logic to check for cycles instead of the recursion depth), I would welcome it, if you could make the value definition of MAX_RECURSION_DEPTH configurable via a parameter. So thanks again for your contribution.
@@ -76,6 +76,7 @@ | |||
//private static final XSImplementation impl = (XSImplementation) new XSImplementationImpl(); | |||
|
|||
private static final XSLoader LOADER; | |||
private static final int MAX_RECURSION_DEPTH = 7; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to provide this attribute/information via a parameter? - since, "7" seems to be rather arbitrary ;)
private Optional<JSElement> iterateElement(final XSElementDeclaration elementDecl) { | ||
|
||
private Optional<JSElement> iterateElement(final XSElementDeclaration elementDecl, int depth) { | ||
if(depth>MAX_RECURSION_DEPTH) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to make this check optional? or to set the default value for MAX_RECURSION_DEPTH to a higher value?
@@ -242,7 +247,7 @@ else if (term instanceof XSModelGroupDefinition) { | |||
}).orElseGet(() -> { | |||
final JSObject jsElements = new JSObject(elementName, isMixed); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I looked into that deeper some time (until I figured out that I will be better off with parsing the rng file rather than the xsd file). In my second attempt I did drop the depth counter and installed bookkeeping for the jsobjects, here. The drawback of this approach is that it introduces state. Do you think that's a better way of fixing that?