thanks a lot for qour hints. I made a few changes based on your tips.

1.) the constructor:
~~~~~~~~~~~
  6         def initialize(path)
  7                 if File.exists?(path) && File.directory?(path) && path!=nil
  8                         @path = path
  9                 else
 10                         puts "Directory doesn't exists"
 11                         exit 1
 12                 end
 13         end
 14         attr_accessor :path
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Does this look OK to you?


2.) program start, object creation
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
27 dir = ARGV.shift
 28 if dir == ['--help'] or dir == ['-h']
 29         puts "Help: ls - list directories and files "
 30 elsif dir then Ls.new(dir).list_all(dir)
 31 else puts "no Argument - define path"
 32 end
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

line 30 is a bit ugly, but I wanted an transparent method signature
for list_all method.
The other thing I thought was to put a help message into the list_all
method in case of more options and more methods like list_files

ben



On Tue, Aug 12, 2008 at 8:39 AM, Stefano Crocco <stefano.crocco / alice.it> wrote:
> On Tuesday 12 August 2008, Ben Aurel wrote:
>> hi
>> In order to learn ruby I'd like to implement some simple unix tools in
>> ruby. I thought I beginn with 'ls':
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>   1 #!/usr/bin/env ruby
>>   2 #
>>   3 # The Unix tool 'ls' implemented in ruby
>>   4 #
>>   5 class Ls
>>   6         attr_accessor :parent_dir
>>   7         def initialize()
>>   8                 puts "Help: ls - list directories and files "
>>   9                 @parent_dir = parent_dir
>>  10
>>  11         end
>>  12         def get_dir(dir)
>>  13                 path = dir
>>  14                 if File.exists?(path) && File.directory?(path) &&
>> path!=nil 15                         return path
>>  16                 else
>>  17                         puts "Directory doesn't exists"
>>  18                         exit 1
>>  19                 end
>>  20         end
>>  21
>>  22         def list_all(dir)
>>  23                 Dir.foreach(dir) do |entry|
>>  24                         if entry != "." || entry != ".."
>>  25                                 puts entry
>>  26                         end
>>  27                 end
>>  28
>>  29         end
>>  30 end
>>  31
>>  32 lister = Ls.new()
>>  33
>>  34 dir = ARGV.shift
>>  35 if dir!=nil
>>  36         lister.list_all(lister.get_dir(dir))
>>  37 else
>>  38         puts "no Argument - define path"
>>  39 end
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> there are a few things that I'm unsure about:
>>
>> 1. the application structure as a whole. Is it ok to test command line
>> argument outside of a class?
>
> Yes (in my opinion, of course).
>
>> 2. the structure of the class itself. Is the constructor
>> (initialization) ok that way?
>
> It depends. Do you want the help message to be displayed every time the
> application is run? If not, then you shouldn't put it into the constructor,
> but use a command line option. For example:
>
> dir = ARGV.shift
> if dir == ['--help'] or dir == ['-h']
>  puts "Help: ls - list directories and files "
> elsif dir then lister.list_all(lister.get_dir(dir))
> else puts "no Argument - define path"
> end
>
> By the way, note that you don't need to write
>
> elsif dir != nil
>
> since everything except nil and false evaluates to true, you can just write
>
> elsif dir
>
>> 3. line 24 doesn't work '.' and '..' are not. How could I do that with
>> regular expressions?
>
> If you don't want to display the '.' and '..' entries, you need to use &&, not
> ||. This is because, when entry is '.', entry != '.' will be false, but
> entry != '..' will be true, so the whole expression will be true ('or' is true
> if at least one operand is true). If you replace || with &&, it will work.
>
> Stefano
>
>