On Nov 3, 2010, at 9:10 PM, Nigel Kersten wrote:
>> + # FIXME: dslocal name is fixed. The default should be "Default" if we do
>> not
>> + # want to break existing installations.
>> + DSLOCAL_DIR = "Default"
>
> If this is a pure refactor with no additional functionality, I'd like
> to see some text explaining why you've refactored the way you did, as
> I'm not entirely convinced that this is all worth the effort.
I refactored the existent code, because I want to do simple feature
changes that other people can follow. I think it is worth to enhance
code quality. I want to undestand parts of code and in this way I do
it, I refactor the code. This is good practice and described in the
"Refactoring" book by Martin Fowler. I try to enhance readabillity of
code that I read, because developers read about 90% of their time code
instead of write code. This was mentioned by Robert C. Martin in the
book "Clean Code".
Class methods are defined within "class << self .. end" like it is a
widely used and well known ruby.
The extrated constant DSLOCAL_DIR is a simple way to extract the hard
coded MCX dslocal database so you can simply change it in the future
to be a member or config whatever it is used in puppet (I do not know
it right now).
For long methods that could be simple split up, I used extract method.
For example I extracted "raise MCXContentProviderException, 'error
message'" into a method error(msg). I should have written that I
changed the Exception fomr MCXContentProviderException to Puppet::Error,
that was my fault not to mention it earlier.
For more readable code I change constructions like "if ! expression"
to "unless expression", because it is better to read.
I deleted some docs that do not give any more information so these docs
can not "lie" in the future.
I deleted not needed temporary variables, because they are useless and
add no readability in these cases.
>> +
>> + class << self
>> + # It returns an array of instances of this class.
>> + def instances
>> + mcx_list = []
>> + TypeMap.keys.each do |ds_type|
>> + ds_path = "/Local/#{DSLOCAL_DIR}/#{TypeMap[ds_type]}"
All the best, Sandor Szücs
--
--
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.
