I have been writing command-line programs using a pattern similar to
described here - http://blog.toddwerth.com/entries/5 . Below is a quick
and dirty code snippet based on this pattern.
#!/usr/bin/env ruby
require ‘ostruct’
require ‘optparse’
class MyPrinter
def initialize(arguments) @arguments = arguments
Set defaults
@options = OpenStruct.new
end
def parse_options
opts_obj = OptionParser.new do |opts|
opts.on(‘-h’, ‘–help’) { @options.help = true }
end
opts_obj.parse!(@arguments)
end
def process_options
output_help if @options.help
end
def output_help
puts ‘Use Google!’
end
def run
parse_options
process_options
end
end
p = MyPrinter.new(ARGV)
p.run
I was wondering if I really need to pass ARGV to MyPrinter’s initialize
method. The ARGV is anyway available to the class so I think it doesn’t
need to be passed and can be used directly during option parsing step -
‘opts_obj.parse!(ARGV)’ . But I am not sure if it’s a good practice or
not. Any tips?
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.
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. 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?
I was wondering if I really need to pass ARGV to MyPrinter’s initialize
method. The ARGV is anyway available to the class so I think it doesn’t
need to be passed and can be used directly during option parsing step -
‘opts_obj.parse!(ARGV)’ . But I am not sure if it’s a good practice or
not. Any tips?
It kind of depends on the scope of what you’re doing. Is it a small one
file script? Then its probably not necessary (of course, in that case,
the
class itself is probably overkill, too, probably a procedural file all
in
the toplevel is enough).
If it’s got more than one file, or you start feeling the need for the
class, then you should probably do it. Otherwise, you’re coupling it to
ARGV, it now knows about who is using it, and it can only be used easily
in
that context. If you want your code to be reusable or testable, then
directly referencing ARGV is a losing strategy. If you can pass in the
args
array and file streams, then you can test that it does everything
correctly, by giving it arrays of hypothetical arguments and StringIO
instances.
In general, it’s better to avoid hard references (references to
constants
and globals). But be aware of your context.
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.
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. 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?
Thanks for the reply Jan. I agree that passing arguments explicitly
makes code cleaner and hence it makes sense to take this approach.
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 was wondering if I really need to pass ARGV to MyPrinter’s initialize
method. The ARGV is anyway available to the class so I think it doesn’t
need to be passed and can be used directly during option parsing step -
‘opts_obj.parse!(ARGV)’ . But I am not sure if it’s a good practice or
not. Any tips?
It kind of depends on the scope of what you’re doing. Is it a small one
file script? Then its probably not necessary (of course, in that case,
the
class itself is probably overkill, too, probably a procedural file all
in
the toplevel is enough).
If it’s got more than one file, or you start feeling the need for the
class, then you should probably do it. Otherwise, you’re coupling it to
ARGV, it now knows about who is using it, and it can only be used easily
in
that context. If you want your code to be reusable or testable, then
directly referencing ARGV is a losing strategy. If you can pass in the
args
array and file streams, then you can test that it does everything
correctly, by giving it arrays of hypothetical arguments and StringIO
instances.
In general, it’s better to avoid hard references (references to
constants
and globals). But be aware of your context.
-Josh
Thanks for the suggestions Josh.
All of my command-line scripts are simple one-class files with structure
similar to the above code snippet. Only few of them use classes and
modules which are outside of the command-line script.
I agree that hard-referencing ARGV is not good idea from reusability and
testing point of view. And I am actually doing some testing (during
development) in this manner without passing ARGV from command-line each
time. So hard-referencing ARGV in this manner won’t be a good idea for
such cases. Thanks for pinting it out.
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
This forum is not affiliated to the Ruby language, Ruby on Rails framework, nor any Ruby applications discussed here.