Floby / node-libspotify

Node bindings for the libspotify C library
projects.flo.by/node-libspotify/
102 stars 66 forks source link

[0.2.1] Wrong-type lookups #8

Closed 19h closed 3 years ago

19h commented 11 years ago

Exceptions should have taken a look at; currently, node dies [from C++ land] when a function of specific type (Album, Artist, Playlist et al.) is used to query a url of different type. (e.g. the album url of Random Access Memories by Daft Punk is used as parameter to query the Artist [Album->Artist]).

We could encounter this issue by obtaining the type of the given url and query its information, when possible. (Album->Artist should be possible, Artist->Album should throw an exception, etc.).

IainCole commented 11 years ago

So basically what you're saying is SpObject.getFromUrl should do a check (with sp_link_type) to make sure that the link is of the correct type before calling linkas(artist/album/etc.)?

19h commented 11 years ago

That's basically it, yes. We could have a flag, though, which would assert an argument to be of the right type (if initialization performance is top-priority in a specific use case).

I didn't say we should only check if a link is of correct type, but, as clever as it could be, automatically obtain the right parameter fitting into a situation if possible. (which is utopistic, as, for instance, a track can have a huge amount of artists; e.g. Symphony No. 9 in D Minor, Op.125 'Choral': Presto 'O Freunde, nicht diese Töne!' with 8 artists given) So basically, we could do a check but, in case we're given a track, we could obtain it's album (if the link to the track is given to the album interface)

Floby commented 11 years ago

Proposed changes:

subject to discussion

19h commented 11 years ago

I'd go with that — except that I would propose an optional flag to SpObject.getFromUrl (that is, also its overrides) preventing a type-lookup, considering cases where lookup performance is top-priority. (and where the type is already known)

Floby commented 11 years ago

I wouldn't got as far as saying type lookups are expensive. It's really just libspotify performin a text search in a 30 characters long string.

Check out what I just pushed to master

On 30 May 2013 12:04, Kenan Sulayman notifications@github.com wrote:

I'd go with that — except that I would propose an optional flag to SpObject.getFromUrl (that is, also its overrides) preventing a type-lookup, considering cases where lookup performance is top-priority.

— Reply to this email directly or view it on GitHubhttps://github.com/Floby/node-libspotify/issues/8#issuecomment-18671628 .

ls12styler commented 11 years ago

I like the API for getAnyFromUrl. However, what would happen in the case of error? eg. couldn't find the type of link provided? Maybe consider the following signature:

getAnyFromUrl(url, function (err, type, data) {}); Or maybe even: getAnyFromUrl(url, function (err, data) {}); // Where data is {type:'album'[, n: 'n']}

Floby commented 11 years ago

Yes, I forgot the error argument. I like (err, type, data).

On 30 May 2013 12:57, Ashley Broadley notifications@github.com wrote:

I like the API for getAnyFromUrl. However, what would happen in the case of error? eg. couldn't find the type of link provided? Maybe consider the following signature:

getAnyFromUrl(url, function (err, type, data) {}); Or maybe even: getAnyFromUrl(url, function (err, data) {}); // Where data is {type:'album'[, n: 'n']}

— Reply to this email directly or view it on GitHubhttps://github.com/Floby/node-libspotify/issues/8#issuecomment-18673531 .

ls12styler commented 11 years ago

In fact, I think from JS dev POV I'd go with having (err, data). This makes the API very similar to things like AJAX libs so users would be familiar with that. Also, is 'type' not part of the data?