Overengineering

[rijkvanzanten]

Hi there!

While I'm quite impressed by the technical prowess of this package, aren't some of the methods a bit over-engineered?

For example, the most basic usage of this package is (as per the readme)

(async () => {
  localStorage.clear();

  const api = new API({
      key: "ADD YOUR KEY HERE"
  });
  const stream = await api.createStream("search/banaan{5}");

  stream
    .pipe(stringify)
    .pipe(console.log)
    .catch(console.error);
})();

however, I feel like the (internal package) API could be way simpler by using a syntax like:

const api = new API({ key: "" });

api.search("banaan")
  .then(console.log)
  .catch(console.error);

you could then provide method shorthands for the available endpoints:

/search
/details
/refine
/schema
/index
/availability
/holdings
/resolver

Again, great job on all the fancy features! I'm a big fan of experimenting with these kind of things, and the minor is the perfect time to do it. I would be the last person to argue otherwise.

That being said, in a real-life use scenario, you don't need all the fancy options. I would even argue that it adds a whole lot of confusion for new users while it doesn't add much in terms of benefits.

[maanlamp]

I'm very much against a separate method for all endpoints (if that is what you're suggesting), which is why I created a small DSL (domain specific language) to select an endpoint and a query with just one method, and I'm not thinking about changing that anytime soon.

As for the overengineering, you're probably right.

That being said, in a real-life use scenario, you don't need all the fancy options. I would even argue that it adds a whole lot of confusion for new users while it doesn't add much in terms of benefits.

The reason I have created three different ways of interfacing with the wrapper, is because I specifically designed it with my co-students in mind. Not everyone knew the concept of streams, iterators, or concurrency/consecutiveness at all. In the end, I probably could have merged all methods into one class, but that makes it less clear that there are several ways to use the wrapper.

I'm thinking of making a v2 that has just one method to interface with the API, and have iterative or streaming functionality in one. The reason I'm keeping all three methods is because they're all very much useful.

Streaming promise resolvements

This makes the requests much faster because it just does all of them at once, and you can call a function per resolvement. This will definately stay.

Iterative resolvements

This is not strictly necessary, but one of the properties of iteration is that it resolves in-order. That's useful in many scenarios, because often you want the order of requests preserverd. The downside of iterators is that it waits for every resolvement before it sends another request. The best of both worlds would be the PromiseStream.pipeOrdered(), since it sends requests concurrently, but resolves in-order. I'm debating removing the iterator and only providing the ordered pipe method.

Native promises

This was just for the completely inexperienced that had just learnt about promises. It turned out to be a useful feature, specifically because of Promise.all, which allowed for code to run after all requests. I'm stripping the raw promise functionality because it's not beneficial, but I'm using them for the PromiseStream.all() method.

Thanks for your input! Very much appreciated.