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

Isn't the suggestion API unnecessarily complicated? #728

Open
veloman-yunkan opened this issue Oct 13, 2022 · 2 comments
Open

Isn't the suggestion API unnecessarily complicated? #728

veloman-yunkan opened this issue Oct 13, 2022 · 2 comments
Assignees
Milestone

Comments

@veloman-yunkan
Copy link
Collaborator

The suggestion API design closely repeats the search API. There are five user-facing classes:

  • SuggestionSearcher produces a SuggestionSearch for the given search text
  • SuggestionSearch allows to specify the desired range from the full list of results and returns the respective subset as a SuggestionResultSet object
  • SuggestionResultSet provides access to individual SuggestionItems via SuggestionIterator

For full-text search such a decomposition makes sense because of the need to support paging of search results. I don't see why we could need that feature for suggestions, and therefore propose to simplify the suggestions API.

We can get along with only to classes:

struct SuggestionItem { ... };

class SuggestionSearcher
{
public:
  explicit SuggestionSearcher(const Archive& archive);
  std::vector<SuggestionItem> suggest(const std::string& query, int maxResults);

...
};

Such an API is much easier to implement in a thread-safe way (the simplest solution being guarding the entry to SuggestionSearcher::suggest() with a mutex).

@mgautierfr
Copy link
Collaborator

SuggestionSearcher is a "clone" of Searcher. Historically, it was the same object (with different parameter). We have split this as it was becoming to difficult to have the same implementation for both. We may indeed rethink the API of suggestion as it is a different usecase.

... because of the need to support paging of search results. I don't see why we could need that feature for suggestions ...

We don't use it in "our" (C++) projects but it is possible that other tools use suggestion API to do search. It was a time where fulltext search was not so useful and people fallback on some search using suggestions.
@automactic, I'm thinking about the MacOs/iOs applications. Can you confirm (or refute) that you using search/suggestions api ?
And if you are using suggestions API, do you use pagination ?

@kelson42 kelson42 added this to the 8.2.0 milestone Oct 19, 2022
@kelson42
Copy link
Contributor

And multizim suggestion has to be handled, see kiwix/libkiwix#479

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

3 participants