Skip to content

Conversation

@archseer
Copy link
Owner

Still messing around with this, I was never satisfied with how the original was implemented, but I didn't want to introduce another dependency back then.

The ideal way probably is to use Hashie::Dash and specify the allowed keys, probably do so dynamically (allowed keys are mpd.types and :file).

@archseer
Copy link
Owner Author

@liquid Yeah, figured I'd rather consult others on this.

Hmm alright, my idea was also to just wrap the Hash object internally and do Forwardable declarations on it.

Main issue I see with all of this is that there's no clear documentation about all of the available fields MPD will return for songs. In general from my experience that's mpd.types + :file, but there might be other fields available (for web streams for example?). Ideally this should just be a generic hash with a few decorator methods and possibly javascript style accessors for the fields.

@whomwah
Copy link
Contributor

whomwah commented Jun 16, 2015

Do you really need to simplify? has it got so out of hand that it needs this change? It's seem quite a simple class at the moment, and not messy.

Personally, I'd much rather see MPD::Song refactored to be made more extendable. For instance, in the project I want to use ruby-mpd I need to be able to extend MPD::Song, so I can pull in extra information based on what is returned from MPD. For example, using the file information and looking up meta data from another data source. I basically like be able to define what I want to use as the MPD::Song class when I initialise the lib. Some sudo code.

class MySongClass < MPD::Song
  def initialize(mpd, options)
    super(mpd, options)
    ...
    # do my stuff
  end
end

mpd  = MPD.new()
mpd.connect
mpd.song_class = MySongClass # uses MPD::Song by default
mpd.foo
...

@mikerodrigues
Copy link
Contributor

I too think Hashie is unnecessary for this. You could do something like this (See Dmitry Polushkin's answer): https://stackoverflow.com/questions/2240535/how-do-i-use-hash-keys-as-methods-on-a-class

I have an example branch here: https://github.com/mikerodrigues/ruby-mpd/tree/mrod-obj-rewrite

**It does seem to always write the first value in the :time array as nil. Hmm.

@archseer
Copy link
Owner Author

archseer commented Nov 8, 2015

@mikerodrigues I do agree that Hashie is a bit of an overkill, your approach looks fine. I'd prefer to not directly subclass Hash though.

We could also use something like OpenStruct?

@archseer
Copy link
Owner Author

archseer commented Nov 8, 2015

At the very least, we should also be defining respond_to? with the current version of the code

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.

5 participants