From: Robert Klemme Date: 2012-07-05T05:58:52+09:00 Subject: Re: passing ARGV to class methods On Wed, Jul 4, 2012 at 2:13 AM, Neubyr Neubyr wrote: > Jan E. wrote in post #1067264: >> You don't have to pass the ARGV to the initialize method, but I'd do it >> anyway, because it shows that the class uses the command line options. >> It simply makes the code clearer. Definitively. BUT: I would not place command line parsing inside the class. Command line processing is something which is only done once, i.e. in the main body of the script. You don't gain anything by placing that in a class because that class isn't reusable. Instead things get more complex than they need to. I prefer to have command line and other option processing outside the classes which do the work and hand over configuration information via property setters. My command line scripts typically look like this foo = "bar" bar = 0 OptionParser.new do |opts| opts.on '--foo' do |v| foo = v end opts.on '--bar=n', 'Doc', Integer do |v| bar = v end end.parse! ARGV ARGF.each do |line| printf "Read line %p\n", line end Which brings me to the next point: >> Apart from that: I think you shouldn't empty ARGV by using >> opts_obj.parse!. Other parts of the program might actually need those >> information. Actually there is a good reason to make exactly that your default coding scheme: by removing options from ARGV only file names remain which makes for nice integration with ARGF. If you parse options twice you're doing something wrong. Options which are parsed should be removed from ARGV also to indicate they have been processed already. >> And I don't like the "run" method, because it isn't clear >> that you can only use it once. Why don't you put it in the initialize >> method? > Also, I liked your suggestion of calling the 'run' method or other > methods in it from the initialize method. Thanks for the extra pointers. I don't. Constructors are for constructing objects and not for doing all the business logic in them. Executing specific logic in a constructor makes a class less versatile. But, as I said above: I find your class MyPrinter superfluous. I'd rather implement all the option processing separately and create classes for the real logic. Kind regards robert -- remember.guy do |as, often| as.you_can - without end http://blog.rubybestpractices.com/