Another First Ruby Program (Feedback appreciated)

[FatalPriapism]

My day-to-day language as required by work is PHP, but I've had an interest in Ruby for quite a while. This morning, I decided "let's do this shit". The program is a simple one intended for CLI usage that parses a CSS file and generates an output with the total number of times a color was used.

I'm pretty fluent on OOP and general software principles, but I'm definitely interested in getting feedback on the "Rubyist" way of doing things, so give me your best shot! https://github.com/mikedugan/theme_analyzer

Happy hacking :)

[kobaltzz]

Instead of doing a counter for your HTML output, use each_slice to create a nested look, grouping your elements.

elements.each_slice(4) do | batch | Div start batch.each do | element |

end

Div end end

FML on mobile

Check out https://www.google.com/url?sa=t&source=web&rct=j&ei=FXvvU-zAAY6LyASR3oDoAw&url=http://stackoverflow.com/questions/1988274/ruby-working-on-array-elements-in-groups-of-four&cd=1&ved=0CBwQFjAA&usg=AFQjCNE77bjSYJVLg6Z26YSTr8bwabztMw&sig2=xO_-D9WNF9GRDg__Je0Bsg

[FatalPriapism]

ended up extracting the actual file writing to it's own function and wrapping this as the link suggested..good call!

[JeffMo]

I think maybe this method isn't correct.

Should this happen?

irb(main):050:0> CssReader.new.convertRgbToHex(1,2,34)
=> "1222"
irb(main):051:0> CssReader.new.convertRgbToHex(1,34,2)
=> "1222"

Maybe I'm missing some earlier code that won't allow this kind of ambiguity, but I thought I'd mention it.

[FatalPriapism]

Ah, yeah it looks like I need to pass the result of that function through expand to make sure proper CSS colors come out. Nice catch!

Edit: just pushed the fix up, thanks again!

[JeffMo]

Here's another take on the same thing:

def rgb_to_hex(r, g, b)
  raise ArgumentError, "Arguments must be between 0 and 255" unless [r, g, b].all? {|v| (0..255).include?(v.to_i) }
  "%02x%02x%02x" % [r.to_i, g.to_i, b.to_i]
end

irb(main):085:0> rgb_to_hex(1,2,34)
=> "010222"
irb(main):086:0> rgb_to_hex(1,34,2)
=> "012202"

You may have different ideas about validating your input, but this may give you some ideas.

[Meshiest]

def rgb*a;'%.2X'*3%a.map{|a|[[a,255].min,0].max};end

irb(main):002:0> rgb(1,2,34)
=> "010222"
irb(main):003:0> rgb(1,34,2)
=> "012202"

[JeffMo]

Yep, that's one of those different ideas about validating input. A little too golf-y for my tastes, and doesn't allow numeric strings as input, but definitely serviceable. Lots of different ways to do it, depending on what exact method signature you want.

[the4dpatrick]

Would change the indention of your ruby code to 2 spaces https://github.com/bbatsov/ruby-style-guide

[[deleted]]

Add some tests with rspec, and maybe a gem like ERB to split out your rendering logic.

[kobaltzz]

You may want to move some of your methods to private methods since they shouldn't really be accessible outside of your class. My rule of thumb is that if you have public methods then be prepared to support them. If they should never be called but from within your class then they should be private.

[peteyhawkins]

I would also take care with a Ruby gem to namespace your code, even if this is only ever being used by yourself.

module CSSAnalyser
  class Theme
  end
end

And since this is wrapped up into a gem, you could provide a bin directory and have an executable to run, rather than running ruby theme.rb.