On 16/09/09 19:36, Markus Roberts wrote: > > > On Wed, Sep 16, 2009 at 8:54 AM, Brice Figureau > <[email protected] <mailto:[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
This isn't equivalent because this code will call match for every regex node instead of just until we find a match. -- Brice Figureau My Blog: http://www.masterzen.fr/ --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
