search

algolia / algoliasearch-client-java

⚡️ A fully-featured and blazing-fast Java API client to interact with Algolia.
https://www.algolia.com/doc/api-client/getting-started/install/java/
MIT License
47 stars 33 forks source link

feat(recommend): add default recommend hit class #762

Closed aallam closed 2 years ago

aallam commented 2 years ago
Q A
Bug fix? no
New feature? no
BC breaks? no
Related Issue n/a
Need Doc update no

Describe your change

Most clients have search methods without hit type, not ideal, but they are there for convenience. This PR adds a default implementation for RecommendHit, and a default recommend search method without hit type to match the other clients.

sbellone commented 2 years ago

I will need some clarifications:

Most clients have search methods without hit type, not ideal, but they are there for convenience.

I understand that you are you talking about API Clients in other languages right? But if they don't have hit types, isn't it because those languages are dynamically typed? Can you give some examples?

This PR adds [...] a default recommend search method without hit type to match the other clients.

It was a design choice to ask users of statically-typed languages to define the type of their Algolia records because the DX is better (and it's actually the philosophy of those languages). Why do we want to change that now? Did we get negative feedbacks? And if we'd want to do it, why only in the Recommend client of the Java API Client?

aallam commented 2 years ago

I understand that you are you talking about API Clients in other languages right? But if they don't have hit types, isn't it because those languages are dynamically typed? Can you give some examples?

Sorry, I was not clear enough, by other clients I meant SearchClient, because of its defaults, it leads to a lot of "typeless" usage of its methods.

It was a design choice to ask users of statically-typed languages to define the type of their Algolia records because the DX is better (and it's actually the philosophy of those languages). Why do we want to change that now? Did we get negative feedbacks? And if we'd want to do it, why only in the Recommend client of the Java API Client?

I totally agree, using "typeless" methods can in many cases lead to bad/unintended behavior. This PR is only to follow the previous "philosophy" of this API client, otherwise, I will not recommend using such an approach.

If we agree that there is no need for such "default" behavior, we can close the PR.

sbellone commented 2 years ago

Thanks for the extra info! I'll have a deeper look today.

sbellone commented 2 years ago

it leads to a lot of "typeless" usage of its methods

Oh interesting. Concretely what are people generally doing? Because by default, a client initialized like that will return SearchResult<Object>. So at some point, I imagine that they have to cast the object into something else? Or is it just to be able to do a System.out.println(results) as fast as possible before working on the typing later?

I ask because I see RecommendClient implementations as something that comes later in the development process, at a stage where developers should already have defined their types properly.

follow the previous "philosophy" of this API client

I'd still like to know better the reason of those usage, but I understand better now! I see that you had only exposed default sync method, was there a reason to not do it for async methods? If we do it, it should probably be done of both.

Finally, I see that you've closed the PR without discussing further, did you change your mind? My goal was really to start the discussion, understand the situation better and find what we can do to improve DX on our statically-typed clients :)