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

change NindLocalAmose::getDocTerms API #3

Open
benlabbe opened this issue Feb 27, 2020 · 1 comment
Open

change NindLocalAmose::getDocTerms API #3

benlabbe opened this issue Feb 27, 2020 · 1 comment

Comments

@benlabbe
Copy link

The method definition and behaviour of NindLocalAmose::getDocTerms() should evolve to embrace the need of corpus migration and slight changes in the configuration of the Amose Server.

The possibility of "not-found" term arise when the corpus and the MediaData configuration evolves : I suggest that the NindLocalAmose::getDocTerms() method avoid any Exception throw because it stops the filling of the end of the termsSet . In the context of highlighted NE display of documents, this early stopping is annoying, whereas one term out of the whole termsSet may be skiped with limited impact.

To conclude, one could change the definition of the method to return an Integer value telling the number of notfound terms. This leaves the handling of this error to the caller , eg amose_nindindex.

See the disabled exception throw in branch getDocTerms_noExcept

Any comment ?

@benlabbe
Copy link
Author

If we have this kind of behaviour in NindLocalAmose::getDocTerms() this might reveal a incoherence in the index files.

I will provide a failing test case, so that an investigation could follow on the potential index coherence error.

Meantime, in the branch getDocTerms_noExcept, the use of a C++11 class enum as the output value of NindLocalAmose::getDocTerms() could serve the three semantic states needed to fill the termsSet as much as possible without raising an exception:

  • OK , all went right
  • NOK_1 , the document is not present
  • NOK_2 , "some" term were not found

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants