openzim / libzim

Reference implementation of the ZIM specification
https://download.openzim.org/release/libzim/
GNU General Public License v2.0
165 stars 49 forks source link

Isn't the suggestion API unnecessarily complicated? #728

Open veloman-yunkan opened 1 year ago

veloman-yunkan commented 1 year ago

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

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 commented 1 year ago

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 commented 1 year ago

And multizim suggestion has to be handled, see https://github.com/kiwix/libkiwix/issues/479