search

ubermichael / isetools

Tools for parsing data for the Internet Shakespeare Editions
GNU General Public License v2.0
2 stars 3 forks source link

stop validators from modifying the DOM #27

Closed telic closed 8 years ago

telic commented 8 years ago

EndNodeValidator and StartNodeValidator were modifying the DOM, causing a java.util.ConcurrentModificationException in ca.nines.ise.validator.DOMValidator#validate, line 81.

Validators should never modify the DOM -- that's what Transformers are for!

ubermichael commented 8 years ago

Thanks Max. That's interesting. I never saw that exception while using it.

telic commented 8 years ago

Would it perhaps make sense for Validators to change the status of the DOM when they detect something wrong? Maybe use DOMStatus.WARNING for this?

ubermichael commented 8 years ago

In this specific case, I think it should be an error. There could be a transformer to fix these things, which builds a new DOM with a clean status. Other transformers (like the RTF transformer, for example) shouldn�t accept a DOM with missing end tags.

telic commented 8 years ago

Right now, DOMStatus.ERROR is only being used for parse errors, and DOMStatus.WARNING is entirely unused. What were your intended use cases for these?

Getting transformers to reset the status could be tricky, since they have nothing but the log to tell them what was actually wrong... It would probably be better to keep status a one-way street (only ever getting worse). Instead, the transformers could be run before the validators, so if they did their job right the status will never get worsened in the first place.

ubermichael commented 8 years ago

Transformers should be building a new DOM, leaving the old one untouched.

I don’t remember what the use cases were.

On Dec 2, 2015, at 1:02 PM, Maxwell Terpstra notifications@github.com wrote:

Right now, DOMStatus.ERROR is only being used for parse errors, and DOMStatus.WARNING is entirely unused. What were your intended use cases for these?

Getting transformers to reset the status could be tricky, since they have nothing but the log to tell them what was actually wrong... It would probably be better to keep status a one-way street (only ever getting worse). Instead, the transformers could be run before the validators, so if they did their job right the status will never get worsened in the first place.

— Reply to this email directly or view it on GitHub.