On Jul 18, 2008, at 2:10 AM, Daniel Pittman wrote:
>
> * Implement Puppet::Parser::Scope#to_hash, which returns a hash
> containing all the
> variable bindings in the current and, optionally, parent scope.
> * Use that to set instance member variables into
> Puppet::Parser::Templatewrapper
> * Report the time taken for variable binding at debug level, to help
> identify any
> performance regression that is encountered in the real world.
> * Rename the @scope member of the template wrapper, to avoid
> clashing with a scope
> variable exposed within puppet.
Looks good.
>
> + # Return a hash containing our variables and their values,
> optionally (and
> + # by default) including the values defined in our parent.
> Local values
> + # shadow parent values.
> + def to_hash(recursive = true, target = Hash.new)
> + if recursive and parent then
> + parent.to_hash(recursive, target)
> + end
> + @symtable.each { |name, value| target[name] = value }
> + return target
> + end
> +
I think it's cleaner to do something like this:
# Using two-space indents because i'm in my mail client, not vim
def to_hash(recursive = true)
if recursive and parent
target = parent.to_hash(recursive)
end
@symtable.each { |name, value| target[name] = value }
return target
end
In other words, don't pass the hash in -- the top-level scope creates
the initial hash, and each sub-scope just merges its data in and
passes it back.
There's at least one bit missing, though; you need something like this
before you return:
target.each { |var, value| target.delete(var) if value == :undef }
I'm always pretty uncomfortable deleting hash members as I iterate, so
maybe something like this:
target.keys.each { |var| target.delete(var) if target[var]
== :undef }
Or even:
return target.inject({}) { |result, ary| result[ary[0]] = ary[1]
unless ary[1] == :undef; result }
Now that I write it out, it doesn't look nearly as cool. :/
Really, though, it probably makes more sense for the scope to return
everything, including the :undef stuff, and just have the
TemplateWrapper strip them out. That way the scope's method is more
generic, and the template wrapper makes it more specific.
>
> # Should return true if a variable is defined, false if it is not
> def has_variable?(name)
> - if @scope.lookupvar(name.to_s, false) != :undefined
> + if scope.lookupvar(name.to_s, false) != :undefined
> true
> else
> false
> end
> end
This method is now more expensive than it needs to be, because we can
easily do something like:
return defined?("@#{name}")
--
Seize opportunity by the beard, for it is bald behind.
-- Bulgarian Proverb
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
