From: Trans <transfire@...>
Date: 2008-08-13T02:17:46+09:00
Subject: Re: what do you think of this code?



On Aug 12, 8:12 am, "Ben Aurel" <ben.au...@gmail.com> 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?

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.