yola / pixabayjs

A Javascript Client for the Pixabay API
MIT License
17 stars 6 forks source link

no way to clear filters or keywords #18

Closed euwest closed 9 years ago

euwest commented 9 years ago

requestFactory uses the same retriever for every request, which uses lodash.assign, which means you can't leave out a property to clear it(you have to know what its default is, and send that to override your previous request).

LionOps commented 9 years ago

It's not a bug, but by design (which isn't to say it's good design). The intent was to create a new RequestFactory for each unique request. The object is poorly named. It should be something like RequestConfigurable or even Request, now that I think of it.

euwest commented 9 years ago

I see. In that case we should definitely rename it

LionOps commented 9 years ago

What should it be named? Request, SearchRequest, Search?

I'm leaning toward Search since its not quite a request yet, and it makes sense to call the client every time there should be a new search.

However, Request also has merit because we could keep the client function as requestFactory and not change the interface. It makes sense to me that calling pixabay.requestFactory hands you a new Request.

Hkly commented 9 years ago

Request seems to make more sense. If you did search there would be a search.search() https://github.com/yola/pixabayjs/blob/master/src/request-factory.js#L22 heh

kahnvex commented 9 years ago

I think the name could be better, it is responsible for creating and returning promises for responses, not requests. We already have a ResultList which is supposed to have an iterable like interface, so I think ResultFactory might make more sense.

We can also discuss a method to make the request factory return a request with fresh params every time. Lets talk about this tomorrow.

LionOps commented 9 years ago

We discussed keeping the interface the same and keeping state on the factory that's given to a retriever when making the ResultList. Won't be too difficult.

kahnvex commented 9 years ago

We discussed keeping the interface the same and keeping state on the factory that's given to a retriever when making the ResultList.

I'm pretty sure that won't clear up the confusion that @euwest and @Hkly brought up.