Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent duplicate stations and seeds (#320) #324

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Raab70
Copy link

@Raab70 Raab70 commented Dec 13, 2017

I am not an expert in objective-C by any means so feel free to tear apart but I wanted to get my feet wet. This addresses #320 by simply returning if a duplicate seed is added. I also noticed that this behavior existed for adding duplicate stations so now if you try to add a duplicate station it will just start playing the existing one.

@nriley
Copy link
Contributor

nriley commented Dec 14, 2017

Thanks, your contribution is appreciated! I haven't had a chance to apply the patch yet but if you could address the issues I mentioned in my review and let me know what a 'duplicate station' is…not sure what you mean here!

@Raab70
Copy link
Author

Raab70 commented Dec 14, 2017

@nriley Not sure what you mean by address the comments from your review, I'm not seeing any comments other than the one above.

To clarify on what I mean by duplicate stations, basically you can follow the steps in #320 but with stations.

  1. Add a station from an artist
  2. That station begins playing
  3. Add another station from the same artist
  4. Now "both" stations are playing in the UI since they point to the same station
  5. Delete either station, it will delete both (or all) of them

This is because it is really just a duplicate reference in the Hermes UI to the same Pandora station. This check just keeps the UI clear. If a user tries to add a station which already exists, it simply takes them to play the station they have which exists.
screenshot 2017-12-13 19 25 35

@Raab70
Copy link
Author

Raab70 commented Dec 14, 2017

Also note that a quit and restart of the app will clean up the station list. The same is true of the seeds list, exiting the Edit Station window and reopening it will clean up the list, this just prevents it from getting into that state.

Sources/Controllers/StationController.m Outdated Show resolved Hide resolved
Sources/Pandora/Pandora.m Outdated Show resolved Hide resolved
Sources/Pandora/Pandora.m Outdated Show resolved Hide resolved
Sources/Pandora/Pandora.m Outdated Show resolved Hide resolved
@nriley
Copy link
Contributor

nriley commented Dec 15, 2017

Thanks for the clarification on the duplicate stations — makes sense. Sorry, it's been a while since I used GitHub's review functionality and I didn't realize I had to submit it before you could see it!

[stations addObject:s];
[Station addStation:s];
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nriley I've cleaned this up a bit, the functionality is the same. Basically the object of this is to skip the addObject and addStation if the station already exists so that we don't get a duplicate in the station list.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants