What do you think of this code?

ruby-talk
1

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?
2. the structure of the class itself. Is the constructor
(initialization) ok that way?
3. line 24 doesn't work '.' and '..' are not. How could I do that with
regular expressions?

thanks in advance for your help and opinion
ben

2

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?

Seems entirely sensible. were you to reuse this class elsewhere you wouldn't give two hoots about the fiddling with ARGV

2. the structure of the class itself. Is the constructor
(initialization) ok that way?

Your constructor doesn't seem to be doing much @parent_dir will only ever be set to nil.
It would make more sense to me if most of the code in get_dir was actually in your constructor, with the list_all method then taking no arguments and listing whatever the relevant instance variable points at.
usage wide your code would then be Ls.new(dir).list_all

3. line 24 doesn't work '.' and '..' are not. How could I do that with
regular expressions?

it's because you need if entry != "." && entry != "..". Every string is going to be not equal to at least one of '.' or '..'

···

On 12 Aug 2008, at 13:12, Ben Aurel wrote:

thanks in advance for your help and opinion
ben

3

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

···

On Tuesday 12 August 2008, Ben Aurel wrote:

4

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?

It's always better to do everything in a class.

2. the structure of the class itself. Is the constructor
(initialization) ok that way?

Did you test it? Where is the local variable parent_dir coming form?
Did you mean

  def intialize(parent_dir)

3. line 24 doesn't work '.' and '..' are not. How could I do that with
regular expressions?

not what? looks okay.

T.

···

On Aug 12, 8:12 am, "Ben Aurel" <ben.au...@gmail.com> wrote:

5

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

6

3. line 24 doesn't work '.' and '..' are not. How could I do that with
regular expressions?

not what? looks okay.

I'm guessing maybe he used a regex like:

if path =~ /..?/

but you need to escape the dots inside regexes if you want to catch
the dot character

if path =~ /\.\.?/

You will also need to check begin and end of the file name using the anchors:

if path =~ /^\.\.?$/

Although I like regexes very much, maybe it is a bit overkill to use
them in this case.

path == "." || path == ".."

looks good for this check.

···

--
Emmanuel Oga
ELC Technologies (TM)
1921 State Street
Santa Barbara, CA 93101
eoga@elctech.com

(866)863-7365 Tel
(866)893-1902 Fax

+44 (0) 20 7504 1346 Tel - London Office
+44 (0) 20 7504 1347 Fax - London Office

7

I like this a lot less. Let's say I want to build a GUI version of the command. I can't use your class now, because it prints text messages and exits when I should probably be showing an error dialog. That makes your class less generally useful, I think.

James Edward Gray II

···

On Aug 12, 2008, at 8:29 AM, Ben Aurel wrote:

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?

8

Yup. Instead of that puts and call to exit I'd raise an appropriate exception (eg ArgumentError)

Your command line harness for this class can always rescue this and print the message.
Fred

···

On 12 Aug 2008, at 14:35, James Gray wrote:

On Aug 12, 2008, at 8:29 AM, Ben Aurel wrote:

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?

I like this a lot less. Let's say I want to build a GUI version of the command. I can't use your class now, because it prints text messages and exits when I should probably be showing an error dialog. That makes your class less generally useful, I think.

9

this is my solution to your inputs

1.) better errorhandling

2.) no more string methods in the class.

···

~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  1 #!/usr/bin/env ruby
  2 # The Unix tool 'ls' implemented in ruby
  3 class Ls
  4 def initialize(path)
  5 if File.exists?(path) && File.directory?(path) && path!=nil
  6 @path = path
  7 else
  8 raise ArgumentError, "Directory doesn't exists"
  9 end
10 end
11 attr_accessor :path
12
13 def list_all_array(dir)
14 files_n_dirs = Array.new
15 Dir.foreach(path) do |entry|
16 if entry != "." && entry != ".."
17 files_n_dirs << entry
18 end
19 end
20 return files_n_dirs
21 end
22 end
23
24 dir = ARGV.shift
25 if dir == ['--help'] or dir == ['-h']
26 puts "Help: ls - list directories and files "
27 elsif dir
28 begin
29 ls = Ls.new(dir)
30 ls.list_all_array(dir).each do |i|
31 puts i
32 end
33
34 rescue ArgumentError => e
35 puts e.message
36 end
37 else puts "no Argument - define path"
38 end
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Like it better now?

On Tue, Aug 12, 2008 at 9:46 AM, Frederick Cheung <frederick.cheung@gmail.com> wrote:

On 12 Aug 2008, at 14:35, James Gray wrote:

On Aug 12, 2008, at 8:29 AM, Ben Aurel wrote:

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?

I like this a lot less. Let's say I want to build a GUI version of the
command. I can't use your class now, because it prints text messages and
exits when I should probably be showing an error dialog. That makes your
class less generally useful, I think.

Yup. Instead of that puts and call to exit I'd raise an appropriate
exception (eg ArgumentError)

Your command line harness for this class can always rescue this and print
the message.
Fred

10

Like it better now?

(for me at least) the point of passing dir to initialize was that list_all would use @path instead of you passing a parameter.

Fred

···

On 12 Aug 2008, at 15:48, Ben Aurel wrote:

On Tue, Aug 12, 2008 at 9:46 AM, Frederick Cheung > <frederick.cheung@gmail.com> wrote:

On 12 Aug 2008, at 14:35, James Gray wrote:

On Aug 12, 2008, at 8:29 AM, Ben Aurel wrote:

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?

I like this a lot less. Let's say I want to build a GUI version of the
command. I can't use your class now, because it prints text messages and
exits when I should probably be showing an error dialog. That makes your
class less generally useful, I think.

Yup. Instead of that puts and call to exit I'd raise an appropriate
exception (eg ArgumentError)

Your command line harness for this class can always rescue this and print
the message.
Fred

11

Hi,

just a short notice: On line 11, you create attribute accessors for @path, i.e. a reader and a writer for @path.
That way, as a user of your class, I can set @path, without
the checks made in initialize(path).

ls = Ls.new(dir)
ls.path=nil
ls.list_all_array -> will not work as expected

So, I would either move
the checks to a "path=" method or remove the write accessor for
@path.

Greetings,

Waldemar

Ben Aurel wrote:

···

this is my solution to your inputs

1.) better errorhandling

2.) no more string methods in the class.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  1 #!/usr/bin/env ruby
  2 # The Unix tool 'ls' implemented in ruby
  3 class Ls
  4 def initialize(path)
  5 if File.exists?(path) && File.directory?(path) && path!=nil
  6 @path = path
  7 else
  8 raise ArgumentError, "Directory doesn't exists"
  9 end
10 end
11 attr_accessor :path
12
13 def list_all_array(dir)
14 files_n_dirs = Array.new
15 Dir.foreach(path) do |entry|
16 if entry != "." && entry != ".."
17 files_n_dirs << entry
18 end
19 end
20 return files_n_dirs
21 end
22 end
23
24 dir = ARGV.shift
25 if dir == ['--help'] or dir == ['-h']
26 puts "Help: ls - list directories and files "
27 elsif dir
28 begin
29 ls = Ls.new(dir)
30 ls.list_all_array(dir).each do |i|
31 puts i
32 end
33
34 rescue ArgumentError => e
35 puts e.message
36 end
37 else puts "no Argument - define path"
38 end
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Like it better now?

12

Hi --

Here are a few more comments.

this is my solution to your inputs

1.) better errorhandling

2.) no more string methods in the class.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 #!/usr/bin/env ruby
2 # The Unix tool 'ls' implemented in ruby
3 class Ls
4 def initialize(path)

Conversion to conventional indentation for free :slight_smile:

5 if File.exists?(path) && File.directory?(path) && path!=nil

File.exists?(nil) will raise a TypeError, so you'll never get to the
final test. If you make it through the first two tests, the third test
is not necessary. So you can get rid of it, and decide whether you
want the nil error to trickle up. The path argument is required,
though, so it's not going to be nil just because someone forgot it; it
will only be nil if someone passes in nil.

You might even consider just storing the path, and then letting
whatever happens later happen -- like someone trying to access a
non-existent or restricted directory.

6 @path = path
7 else
8 raise ArgumentError, "Directory doesn't exists"
9 end
10 end
11 attr_accessor :path
12
13 def list_all_array(dir)
14 files_n_dirs = Array.new
15 Dir.foreach(path) do |entry|
16 if entry != "." && entry != ".."
17 files_n_dirs << entry
18 end
19 end

A slightly more straightforward way to handle that might be:
   next if ['.', '..'].include?(entry)
   files_n_dirs << entry

20 return files_n_dirs
21 end
22 end
23
24 dir = ARGV.shift
25 if dir == ['--help'] or dir == ['-h']
26 puts "Help: ls - list directories and files "
27 elsif dir
28 begin
29 ls = Ls.new(dir)
30 ls.list_all_array(dir).each do |i|
31 puts i
32 end

You can just do:

   puts ls.list_all_array(dir)

David

···

On Tue, 12 Aug 2008, Ben Aurel wrote:

--
Rails training from David A. Black and Ruby Power and Light:
  * Advancing With Rails August 18-21 Edison, NJ
  * Co-taught by D.A. Black and Erik Kastner
See http://www.rubypal.com for details and updates!

13

hi
I'm hugely impressed by your helpfull comments. Thanks alot. I've
tried to follow your recommondations and made again a new version. I
thinks its a cool little program, and I think I post it somewhere with
comments. As reminder for me and maybe valuable information for others

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~`
#!/usr/bin/env ruby
# The Unix tool 'ls' implemented in ruby
class Ls
  @path = ""
  def initialize(path)
    if File.exists?(path) && File.directory?(path)
      @path = path
    else
      raise ArgumentError, "Directory doesn't exists"
    end
  end
    
  def list_all()
    files_n_dirs = Array.new
    Dir.foreach(@path) do |entry|
      next if ['.','..'].include?(entry)
      files_n_dirs << entry
    end
    return files_n_dirs
  end
end

dir = ARGV.shift
if dir == ['--help'] or dir == ['-h']
  puts "Help: ls - list directories and files "
elsif dir
  begin
    puts Ls.new(dir).list_all()
  rescue ArgumentError => e
    puts e.message
  end
else puts "no Argument - define path"
end

···

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

On Tue, Aug 12, 2008 at 1:45 PM, David A. Black <dblack@rubypal.com> wrote:

Hi --

Here are a few more comments.

On Tue, 12 Aug 2008, Ben Aurel wrote:

this is my solution to your inputs

1.) better errorhandling

2.) no more string methods in the class.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 #!/usr/bin/env ruby
2 # The Unix tool 'ls' implemented in ruby
3 class Ls
4 def initialize(path)

Conversion to conventional indentation for free :slight_smile:

5 if File.exists?(path) && File.directory?(path) && path!=nil

File.exists?(nil) will raise a TypeError, so you'll never get to the
final test. If you make it through the first two tests, the third test
is not necessary. So you can get rid of it, and decide whether you
want the nil error to trickle up. The path argument is required,
though, so it's not going to be nil just because someone forgot it; it
will only be nil if someone passes in nil.

You might even consider just storing the path, and then letting
whatever happens later happen -- like someone trying to access a
non-existent or restricted directory.

6 @path = path
7 else
8 raise ArgumentError, "Directory doesn't exists"
9 end
10 end
11 attr_accessor :path
12
13 def list_all_array(dir)
14 files_n_dirs = Array.new
15 Dir.foreach(path) do |entry|
16 if entry != "." && entry != ".."
17 files_n_dirs << entry
18 end
19 end

A slightly more straightforward way to handle that might be:
next if ['.', '..'].include?(entry)
files_n_dirs << entry

20 return files_n_dirs
21 end
22 end
23
24 dir = ARGV.shift
25 if dir == ['--help'] or dir == ['-h']
26 puts "Help: ls - list directories and files "
27 elsif dir
28 begin
29 ls = Ls.new(dir)
30 ls.list_all_array(dir).each do |i|
31 puts i
32 end

You can just do:

puts ls.list_all_array(dir)

David

--
Rails training from David A. Black and Ruby Power and Light:
* Advancing With Rails August 18-21 Edison, NJ
* Co-taught by D.A. Black and Erik Kastner
See http://www.rubypal.com for details and updates!

14

#!/usr/bin/env ruby
# The Unix tool 'ls' implemented in ruby
class Ls
       @path = ""

This is not doing what you think it does - when @path = "" is
executed, you are in the context of the class object Ls, which will
acquire a "class instance variable" @path, which is then never used
(or, indeed, accessible). If your intent was to have a default path of
"" if none was given, you need the next line to be

       def initialize(path)

def initialize(path = "")

but since immediately after you are checking the path

               if File.exists?(path) && File.directory?(path)

you might as well let it be nil if not supplied, and omit the default
altogether.

martin

···

On Tue, Aug 12, 2008 at 4:37 PM, Ben Aurel <ben.aurel@gmail.com> wrote:

15

This is not doing what you think it does - when @path = "" ...

thanks for the input. Here are my changes and my reasoning about it:

- like the unix 'ls' command the default parameter is '.'

- I demand 'path' as a initialize argument but I wont set a default
value because creating a new instance with new() is wrong and should
raise an error.

- I try to catch all the important exceptions

···

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  4 def initialize(path)
  5 @path = path
  6 if @path == "" || @path == nil
  7 @path = "."
  8 elsif File.exists?(@path)
  9 raise ArgumentError, "This is a file not a
directory"
10 elsif !File.directory?(@path)
11 raise ArgumentError, "Directory doesn't exists"
12 end
13 end
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I have to admit that I have a bit a hard time to grock the usage of
the different ruby class and object variables (local vars, global,
instance, class). I hope correct this time (line 5)

On Tue, Aug 12, 2008 at 7:46 PM, Martin DeMello <martindemello@gmail.com> wrote:

On Tue, Aug 12, 2008 at 4:37 PM, Ben Aurel <ben.aurel@gmail.com> wrote:

#!/usr/bin/env ruby
# The Unix tool 'ls' implemented in ruby
class Ls
       @path = ""

This is not doing what you think it does - when @path = "" is
executed, you are in the context of the class object Ls, which will
acquire a "class instance variable" @path, which is then never used
(or, indeed, accessible). If your intent was to have a default path of
"" if none was given, you need the next line to be

       def initialize(path)

def initialize(path = "")

but since immediately after you are checking the path

               if File.exists?(path) && File.directory?(path)

you might as well let it be nil if not supplied, and omit the default
altogether.

martin

16

thanks for the input. Here are my changes and my reasoning about it:

- like the unix 'ls' command the default parameter is '.'

- I demand 'path' as a initialize argument but I wont set a default
value because creating a new instance with new() is wrong and should
raise an error.

This doesn't make sense to me. Why shouldn't you be able to create a
new instance without an argument and have it default to the current
directory?
System 'ls' defaults to the current directory. I think I shoud be
able to type "ls.rb" without arguments and get the current directory
list, not a "No Argument" warning.
To make it work like that, I would use a default argument,

4 def initialize(path = '.')
5 @path = path.empty? ? '.' : path
8 if File.exists?(@path)

...

and remove the 'elsif dir' clause before the begin block at the end of
your script.

Also, a standard idiom for files that can be either run from the
command line or required is to put this test around the command line
handling part:

  if __FILE__ == $0
  end

That condition will only be true if this file is being executed
directly, not when it is 'require'd.

-Adam

···

On 8/13/08, Ben Aurel <ben.aurel@gmail.com> wrote: