ruby-mpd
ruby-mpd copied to clipboard
WIP: Introduce hashie to simplify object wrappers.
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
).
@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.
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
...
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 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
?
At the very least, we should also be defining respond_to?
with the current version of the code