critique please

[Charles Oppenheimer]

Folks, the first ruby I've written that I actually needed (I shortened
the actual strings I was comparing, which were very long). So
thought I'd ask, before I write the next one - is there a more elegant
way to do this, or conventions that are usually used?


Thanks!

globalcampaigns = "item1, item2, item3, item4, item5"
premiercampaigns = "item1, item2"
basiccampaigns = "item3, item4"


globalarray = globalcampaigns.split(",")
premarray = premiercampaigns.split(",")
basicarray = basiccampaigns.split(",")

#globalarray.each {|elt| puts elt}

combo = premarray | basicarray
diff = globalarray - combo

diff.each {|elt| puts elt}

 

[Ryan Davis]

Folks, the first ruby I've written that I actually needed (I shortened
the actual strings I was comparing, which were very long). So
thought I'd ask, before I write the next one - is there a more elegant
way to do this, or conventions that are usually used?

your code looks fine.

convention-wise, I'd recommend using underscores to make your variables =
more idiomatic and readable, eg: global_campaigns. I'd also recommend =
off of naming things with types ala hungarian notation and friends. =
Usually the fact that the variable is plural is sufficient to show it is =
some sort of collection.

 

[Stu]

If you want to save yourself a step you can use the %w{ }

global_campaigns =3D %w{ item1 item2 item3 item4 item5 }

~Stu

 

[Chris Kottom]

[Note: parts of this message were removed to make it a legal post.]

Also the split statements you've used will cause the spaces between the
commas and the "item" strings to be preserved in the resulting arrays which
is probably not the desired effect.

ruby-1.9.2-p0 :001 > globalarray = globalcampaigns.split(",")
=> ["item1", " item2", " item3", " item4", " item5"]


To get around this, pass a regular expression with optional leading and
trailing whitespace as the parameter to split.

ruby-1.9.2-p0 :002 > globalarray = globalcampaigns.split(/\s*,\s*/)
=> ["item1", "item2", "item3", "item4", "item5"]

If you want to save yourself a step you can use the %w{ }

global_campaigns = %w{ item1 item2 item3 item4 item5 }

~Stu

 

[Robert Klemme]

Folks, the first ruby I've written that I actually needed (I shortened
the actual strings I was comparing, which were very long). So
thought I'd ask, before I write the next one - is there a more elegant
way to do this, or conventions that are usually used?


Thanks!

globalcampaigns = "item1, item2, item3, item4, item5"
premiercampaigns = "item1, item2"
basiccampaigns = "item3, item4"


globalarray = globalcampaigns.split(",")
premarray = premiercampaigns.split(",")
basicarray = basiccampaigns.split(",")

#globalarray.each {|elt| puts elt}

combo = premarray | basicarray
diff = globalarray - combo

diff.each {|elt| puts elt}

If those are large I'd use Set:

require 'set'

globalcampaigns = "item1, item2, item3, item4, item5"
premiercampaigns = "item1, item2"
basiccampaigns = "item3, item4"

res = globalcampaigns.split(/,/).to_set.
subtract(premiercampaigns.split(/,/).to_set).
subtract(basiccampaigns.split(/,/).to_set)

puts res.to_a

Note that there are some issues with regard to whitespace.

You could also do

gc = globalcampaigns.split(/\s*,\s*/).to_set

[premiercampaigns, basiccampaigns].each do |str|
str.scan /[^,]+/ do |item|
gc.delete item.strip
end
end

puts gc.to_a

Kind regards

robert

 

[Charles Oppenheimer]

Awesome, thanks for the tips. Actually the white space was I accidently
added added when I manually changed from the orginal (which was too long
to post)... then I ran the modified I noticed it didn't work (without
regular expressions).

I did find one shortcut, by adding .split to the end of the string saved
a few lines... Also, its something I couldn't have done in Perl!
Although in perl I could have created the array with @array = split(',',
"item1,item2")

Anyway, I'm sold! Lovin ruby.

globalcampaigns = "item1,item2,item3,item4,item5".split(",")
premiercampaigns = "item1,item2".split(",")
basiccampaigns = "item3,item4".split(",")

combo = premiercampaigns | basiccampaigns
diff = globalcampaigns - combo

diff.each {|elt| puts elt}