On Mon, Feb 14, 2011 at 16:36, Paul Berry <[email protected]> wrote:
> The logic for marking inspect reports with an appropriate icon is now
> in a callback method that plugs into :report :status_icon, rather than
> being hardcoded in the _report_status_icon partial. This paves the
> way for additional future modularity in choosing report icons.
[…]
> + = icon(Registry.find_first_callback(:report, :status_icon) { |thing|
> thing.call(report) } || report.status)
The more I see of it, the less I like this idiom. It seems very
convoluted to be invoking the block indirectly to have it invoke the
callback. Is there an example of where this is used for *anything*
but invoking blocks?
I would much prefer to see it reduced to eliminate the block, but I am
open to being persuaded that there is value by seeing where it ends up
with significantly simpler code than the much more obvious:
=icon(Registry.invoke(:report, :status_icon, report) || report.status)
...with the (I think) obvious semantics: invoke the report status_icon
hook, pass the report as the argument to the hook functions, and use a
default if nil comes back from the hooks.
Assuming that I actually understood the semantics correctly, and the
behaviour isn't actually "get the first callback, try invoking it, and
if it returns nil then use the value of report.status, unless that is
nil, in which case do the same to the second callback". Which it
could actually be, if me reading of the code is right, which I
*really* hope it isn't.
Daniel
--
⎋ Puppet Labs Developer – http://puppetlabs.com
✉ Daniel Pittman <[email protected]>
✆ Contact me via gtalk, email, or phone: +1 (877) 575-9775
♲ Made with 100 percent post-consumer electrons
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
