On 12 Aug 2008, at 15:48, Ben Aurel wrote:
>
>
> 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 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
>>
>>
>