-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Some stuff I noticed in the code that I collected as tips :-)
- - obj == nil
-> obj.nil?
- - str.length > 0
-> str.empty?
- - if str && str.length > 0
This is tricky, not only because str.length should be tested with
empty, but this test is so common in web forms, that rails adds the
method Object#blank? so one can test nil? and empty? in one shot.
-> str.blank?
- - comments next to 'end', if we need to comment next to an end to give a
hint to which block it belongs, it means we have to refactor that code
and split it/un-nest it. If you are commenting an end block, you need
then to feel guilty. Your face has to go red and you make a concrete
appointment to fix that code ;-)
- - Something like:
if not @user.home_directory.blank?
command << "home...@user.home_directory}"
end
(already refactored to use blank?)
can be refactored to:
command << "home...@user.home_directory}" if not
@user.home_directory.blank?
This is not very important, but if you have 20 of those blocks in a
row, the continuous starts and end of one line blocks make code too
verbose and eyes have to indent and unindent all the time, while a one
liner reads as english.
- - Use and learn stdlib methods:
if not @user.groups.blank?
counter = 0
grp_string = ""
@user.groups.each do |group|
if counter == 0
grp_string = group[:id]
else
grp_string += ",#{group[:id]}"
end
counter += 1
end
command << "grouplist=#{grp_string}"
end
What are we doing here? just converting an array into a string, but
with commas. So first we are duplicating Array#join.
I guess the code does that because it has a collection of Group
objects, and it want to create an array of ids of those groups.
You can get [1,2,3..] as the array of the group ids by using Array#map:
groups.map { |group| group[:id] }. Add Array#join to the equation:
groups.map { |group| group[:id] }.join(',')
and you have all the code above in one line! :-) it returns "1,2,3" as
a String.
I fixed most of the issues mentioned, but sure there are more. Fix them
as you spot them.
cheers
Duncan
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org
iEYEARECAAYFAkn0uTQACgkQzR62qWZ+QtEkLgCfVSPfrWXY88ZBG3Wrxj5b/Hnr
pKEAoJTQPD9/+3DF27rQa1KVNoOxp6ke
=erdq
-----END PGP SIGNATURE-----
--
To unsubscribe, e-mail: yast-devel+unsubscr...@opensuse.org
For additional commands, e-mail: yast-devel+h...@opensuse.org
