ruby-mpd icon indicating copy to clipboard operation
ruby-mpd copied to clipboard

WIP: Introduce hashie to simplify object wrappers.

Open archseer opened this issue 9 years ago • 5 comments

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 avatar Jun 15 '15 11:06 archseer

@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.

archseer avatar Jun 16 '15 09:06 archseer

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
...

whomwah avatar Jun 16 '15 14:06 whomwah

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.

mikerodrigues avatar Nov 03 '15 02:11 mikerodrigues

@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 avatar Nov 08 '15 17:11 archseer

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

archseer avatar Nov 08 '15 17:11 archseer