search

zmb3 / spotify

A Go wrapper for the Spotify Web API
Apache License 2.0
1.34k stars 284 forks source link

GetArtistAlbumsOpt: Improvements and bug fixes #116

Closed brodin closed 4 years ago

brodin commented 4 years ago

This API will be backwards compatible, but also support the full potential of the underlying endpoint. See https://developer.spotify.com/documentation/web-api/reference/artists/get-artists-albums/ for reference

zmb3 commented 4 years ago

Thanks!

brodin commented 4 years ago

@zmb3 I found an issue with this. If you explicitly send in nil as album type this will crash. This since ts then will have length 1 and this loop will result in a nil-pointer https://github.com/zmb3/spotify/pull/116/files#diff-234075626fc63bfff8ccab16d40509e9R124

So the code is not fully backwards compatible. Do you want me to add a guard for it?

The guard would be to change this line https://github.com/zmb3/spotify/pull/116/files#diff-234075626fc63bfff8ccab16d40509e9R121 to

if ts != nil && ts[0] != nil then we should be fully backwards compatible.

zmb3 commented 4 years ago

@brodin sure. Note that you don't even need the if ts != nil check, as you can range over a nil slice, and the loop simply won't execute.