From: Robert Klemme Date: 2013-03-09T22:02:50+09:00 Subject: Re: RubyExcel class. Useful? On Fri, Mar 8, 2013 at 4:49 PM, Joel Pearson wrote: > I've managed to create a (relatively) stable data-processing class which > behaves a bit like Excel, but with some of Ruby's awesome syntax styles. > I've used it on a few projects now so I'm happy with how it works, > although I'm always trying to improve it. Good, I see you're making progress! One thing I noticed on quick glancing is that you are overusing string interpolation. self =~ "#{s}" -> self =~ s.to_s if "#{ val }".empty? -> if val.to_s.empty? In []= you are evaluating the same condition twice. I also share Andrew's feeling that you might get away with less code. Somehow it feels like you put too much into the class. For example, methods #strip! and #upcase! deal with individual cell manipulations. They may make sense as convenience methods but would first try to focus on the core functionality of the spreadsheet: inserting and removing rows and columns. And method #test does not belong there this should go into a test class. Another thing that stroke me odd is that you assume there will always be one header row. IMHO that is a too specific assumption. If any I would only put this in a subclass or a wrapper class. Finally I believe you are working too much with Arrays. For example in method #+ instances of Row, Column and RubyExcel are converted to Arrays before they are appended. Since these classes belong to this library you know how their internals look and you should have a better way to transfer their data over to this sheet. Same for #- and probably others as well. Kind regards robert -- remember.guy do |as, often| as.you_can - without end http://blog.rubybestpractices.com/