On Wed, Sep 16, 2009 at 8:54 AM, Brice Figureau <
[email protected]> wrote:
>
> On Wed, 2009-09-16 at 07:47 -0700, Markus wrote:
> > On Wed, 2009-09-16 at 09:49 +0200, Brice Figureau wrote:
> > > On Tue, 2009-09-15 at 15:16 -0700, Luke Kanies wrote:
> > > > I can't think of a better solution, but I'm not real stoked about
> > > > 'node_exists?' returning something other than a boolean.
> > >
> > > Yes, I had the same question. But couldn't find a better name which
> > > didn't involve renaming all other call sites. I tried node_by_name or
> > > node_by_equality, but none where really satisfying either.
> >
> > Rename any call sites that use the return value of 'node_exists?' as
> > anything other than a boolean. If this is all of them, then the method
> > should be renamed. If this is only some of them, then there are really
> > two methods, one of which (probably 'node_exists?') is a thin wrapper on
> > the other.
>
> Yes, the method should be renamed. I suck at naming things. When I sent
> the patch (crafted in a couple of minutes), I knew I'd have some
> objection.
>
> My issue is that there are lots of clients of "node" (which in fact does
> a the job of "match_node"), and since there is only one client of
> "node_exists?" this was the simplest solution.
> And I don't have much time to devote to Puppet coding before the 0.25.1
> release, so I'd prefer to find a good name for node_exists? instead of
> doing the reverse (ie s/node/match_node/g).
>
> Now, since I suck at naming things, can someone give me alternative
> names for this method?
>
> > There's no reason a method like 'foo?' can't return any reasonable value
> > it likes, but you should only count on the boolean traits of the result
> > (if you want to force it to be a boolean you can always use the '!!foo?'
> > idiom).
> >
> > I'm planning some code-quality sweeps, and this is one of the target
> > issues.
>
> It will be interesting to see the results.
>
I'd probably write something like:
def node_called(name)
@nodes[check_name(name)]
end
def node_matching(name)
node_called(name) || @node_list.select { |n| n.regexp? and
n.match(name)}.find { |n| @nodes[n] }
end
Where node_matching() replaced the old node() and node_called() replaced
node_exists().
-- Markus
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
