class Item
def raw_tag_list; "one, two, three, four, five"; end
def public_tags; [<#Tag name=two>, <#Tag name=five>, <#Tag name=one>, <#Tag name=four>, <#Tag name=three>]; end
def tags
return Array.new if raw_tag_list.nil?
# Get the raw tag list, split, squish (removed whitespace), and add each to raw_tag_array
# Make sure we skip if the array already has that tag name (remove any duplicates that occur)
raw_tag_array = Array.new
raw_tag_list.split(',').each do |raw_tag|
next if raw_tag_array.include?(raw_tag.squish)
raw_tag_array << raw_tag.squish
end
tags = Array.new
# grab all the tag objects
tags_out_of_order = public_tags
if tags_out_of_order.size > 0
# resort them to match raw_tag_list order
raw_tag_array.each do |tag_name|
tag = tags_out_of_order.select { |tag| tag.name == tag_name }
tags << tag
end
# at this point, we have an array, with arrays of object [[tag], [tag], [tag]]
# use compact to remove any nil objects, and flatten to convert it to [tag, tag, tag]
tags = tags.compact.flatten
end
tags
end
end
Refactorings
No refactoring yet !
Tj Holowaychuk
March 30, 2009, March 30, 2009 23:40, permalink
I dont know exactly what your trying to accomplish, but its wrong :p
class Tag
def initialize name
@name = name
end
def self.parse string
string.split(',').map { |tag| Tag.new(tag.strip) }
end
end
params = { :tags => 'foo,bar, some more bar, whoop' }
p Tag.parse(params[:tags])
# => [#<Tag:0x27a88 @name="foo">, #<Tag:0x27a60 @name="bar">, #<Tag:0x27a38 @name="some more bar">, #<Tag:0x27a10 @name="whoop">]
k776
March 30, 2009, March 30, 2009 23:46, permalink
Thanks for trying. I ended up going with the following or now. Note the sort replacing 5 lines. The idea is to take an unordered sql query result from Rails, and order it according to a comma separated string.
class Item
def raw_tag_list; "one, two, three, four, five"; end
def public_tags; [<#Tag name=two>, <#Tag name=five>, <#Tag name=one>, <#Tag name=four>, <#Tag name=three>]; end
def tags
return Array.new if raw_tag_list.nil?
# Get the raw tag list, split, squish (removed whitespace), and add each to raw_tag_array
# Make sure we skip if the array already has that tag name (remove any duplicates that occur)
raw_tag_array = Array.new
raw_tag_list.split(',').each do |raw_tag|
next if raw_tag_array.include?(raw_tag.squish)
raw_tag_array << raw_tag.squish
end
tags = Array.new
# grab all the tag objects
tags_out_of_order = public_tags
if tags_out_of_order.size > 0
# resort them to match raw_tag_list order
tags = tags_out_of_order.sort { |a, b| raw_tag_array.index(a.name).to_i <=> raw_tag_array.index(b.name).to_i } ### <-----
end
tags
end
end
k776
March 30, 2009, March 30, 2009 23:47, permalink
(credits to k1mmeh at http://snippets.dzone.com/posts/show/6608 for the one line optimization)
Tj Holowaychuk
March 31, 2009, March 31, 2009 00:12, permalink
still pretty messy IMO
k776
March 31, 2009, March 31, 2009 06:41, permalink
Got it down to this. Can you do better?
class Item
def raw_tag_list; "one, two, three, four, five"; end
def public_tags; [<#Tag name=two>, <#Tag name=five>, <#Tag name=one>, <#Tag name=four>, <#Tag name=three>]; end
def tags
return Array.new if raw_tag_list.blank?
raw_tag_array = raw_tag_list.squish.split(/\s?,\s?/).uniq
tags = public_tags
tags.sort! { |a, b| raw_tag_array.index(a.name).to_i <=> raw_tag_array.index(b.name).to_i } if tags.size > 0
tags
end
end
Tj Holowaychuk
March 31, 2009, March 31, 2009 15:15, permalink
Just seems like a really hackish thing to do even having hardcoded orders like that, and sorting like that. Maybe consider using the Comparable mixin or abstract the sorting to a different method. Squishing it all into #tags will just slow it down and prevent you from using that functionality later
Adam
April 1, 2009, April 01, 2009 05:07, permalink
require 'Linguistics'
class Tag < Struct.new(:name)
NUMBERS = (0..100).inject(Hash.new(1 / 0.0)) do |hash,number|
hash.merge(Linguistics::EN.numwords(number) => number)
end
def to_i
NUMBERS[name]
end
def <=>(tag)
to_i <=> tag.to_i
end
end
class Item
def public_tags
[ Tag.new('eight'), Tag.new('five'), Tag.new('ten'), Tag.new('foo'), Tag.new('one') ]
end
def tags
public_tags.sort
end
end
Tj Holowaychuk
April 1, 2009, April 01, 2009 15:19, permalink
Not a huge fan of people using Struct all the time, adding another class to the chain JUST to add one attribute is pretty bad practice IMO, but at least you were able to figure out what he wanted lol
Adam
April 1, 2009, April 01, 2009 15:39, permalink
Refactored to eliminate the inheritance because Tj doesn't like it extra classes. ;)
require 'Linguistics'
Tag = Struct.new(:name).class_eval do
NUMBERS = (0..100).inject(Hash.new(1 / 0.0)) do |hash,number|
hash.merge(Linguistics::EN.numwords(number) => number)
end
def to_i
NUMBERS[name]
end
def <=>(tag)
to_i <=> tag.to_i
end
self
end
class Item
def public_tags
[ Tag.new('eight'), Tag.new('five'), Tag.new('ten'), Tag.new('foo'), Tag.new('one') ]
end
def tags
public_tags.sort
end
end
Tj Holowaychuk
April 1, 2009, April 01, 2009 16:23, permalink
Haha :P sorry just seems really needless, thats the reason for the attr_* methods
Adam
April 1, 2009, April 01, 2009 16:31, permalink
I like to make extensive use of Struct on refactormycode because it hides boring implementation details, letting the reader focus on the actual refactoring.
Tj Holowaychuk
April 1, 2009, April 01, 2009 17:47, permalink
True good point, ive just seen it lots in production code as well with huge arrays of symbols looking all nasty
Is there a faster way to do this? I need to sort an array of tags according to the order of tags in a string. At the moment, I make the array then select as go through. But this is slow. Is there a fancy ruby alternative?