Skip to content

JENA-1252: Tokenizer cleaup#184

Merged
asfgit merged 3 commits intoapache:masterfrom
afs:tokenizer-cleaup
Oct 28, 2016
Merged

JENA-1252: Tokenizer cleaup#184
asfgit merged 3 commits intoapache:masterfrom
afs:tokenizer-cleaup

Conversation

@afs
Copy link
Copy Markdown
Member

@afs afs commented Oct 27, 2016

Puts in place the structure for warnings not being tokenizer errors.

This is not enabled - it needs testing to see what knock-on effects it has. Ideally, it should be combined with parsers that can do some level of error recovery.

https://issues.apache.org/jira/browse/JENA-1252

private TokenChecker checker = null ;

// The code assumes that errors throw exception and so stop parsing.
private ErrorHandler errorHandler = new ErrorHandler() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I may be misunderstanding here, but couldn't this be static?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NM, I see the mutator and accessor below. Still, the default instance could be factored out into a constant, I think.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will do. It is stateless though a stateless object is not very expensive to create!

The others are in ErrorHandlerFactory.

It maybe it becomes a parameter to the constructor.

Because letting dubious data through can lead to a lot of support load due to downstream database problems, this PR just adds mechanism and does not enable it. Hence it is keeping the default error handler in-file for now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍 to the scheme (not making it too easy to let cruft through).

@asfgit asfgit merged commit e11f185 into apache:master Oct 28, 2016
@afs afs deleted the tokenizer-cleaup branch October 28, 2016 15: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.

3 participants