> Dermot wrote:
>> 2008/8/11 Rob Dixon <[EMAIL PROTECTED]>:
>>
>>> - The usual reason for using a variable name with a leading underscore is to
>>> denote that it's a private variable, but lexical variables aren't visible
>>> outside the package anyway so you may as well drop the underscore and make
>>> it
>>> more readable. You should also call it something better than HASH because
>>> the %
>>> says that, so how about %TYPES?
>>
>> I think John K had a pop at me for doing this once before. What I've
>> done is change the name,% _HASH in my code actually reads %_KNOWNTYPES
>> but I take the point that it's scoped and I don't have to make it read
>> any uglier.
>
> An informed decision is all we expect :)
>
>>> - It's very unusual to bless an array reference for use as an object. I
>>> think
>>> you should use a hash reference, and if you have array-like core data just
>>> make
>>> one of the hash values an anonymous array.
>>
>> There is an OO book by Conway and I read an abridged version that
>> suggested this array of hash structure because array access can be
>> faster. One of the elements in the hash, during testing/development at
>> least is an array of 300 items.
>
> I feel very strongly that software should be written primarily for
> readability.
> If a highly-legible and fully-functional program proves to run too slowly,
> that
> it the time to make changes to improve the speed. One of the delights of
> object
> orientation is encapsulation, which lets you switch between data
> representations
> freely as long as the the interface remains constant. I suggest you base your
> implementation on a hash reference for convenience and readability, and switch
> to an array reference if you need to make things work faster.
>
> In any case, I'm certain that Conway didn't mean that things would be faster
> if
> you simply injected an extra level of indirection. $self->{type} will always
> be
> quicker than $self->[0]{type}. I guess what was intended was that an array
> access is faster than a hash reference (and will consume less memory) so you
> could write
>
> use constant TYPE => 0;
>
> my $type = $self->[TYPE];
>
> and so on.
>
>> It doesn't look like my code with exceed 600 lines. Randal suggests
>> that code should be > 1000 lines before you see any benefits in using
>> OO.
>
> That is a rule of thumb, and each application has its own values to define
> 'benefit'. Here, for example, you should count in the advantage of learning
> object-oriented coding. Object orientation is usually a trade-off of clarity
> and
> reusability of code over execution speed (an equivalent OO solution is
> genrally
> slower). But you must factor in your own aims and values.
>
>
>>
>> sub append_log {
>>
>> my $fh;
>> open $fh, ">>$self->[0]->{type}->{logfile}" or die "Cannot append: $!\n";
>> $self->[1] = $fh;
>> }
>>
>> and later
>>
>> select $self->[1]
>
> Now you're beginning to use the array for speed as I think it was intended by
> Conway. But look at the cost! Wouldn't it be much nice to be able to write
>
> select $self->{filehandle}
>
> for now?
>
> If I was stuck with the data structure I would quickly replace the 'real'
> self,
> something like this
>
> sub get_type {
> my $self = shift->[0];
> return $self->{type}
> }
>
>>
>> Personally I find the arrow more readable.
>
> Fine. Use what you feel best with.
>
>>
>> Gosh if $self's type could change that would really screw things up!!!
>
> OK. As I thought. So you need a get_type method that only reads the value.
>
>>>
>>> sub type {
>>>
>>> my $self = shift;
>>>
>>> if (@_) {
>>> my $newtype = shift;
>>> $self->{type} = $newtype;
>>> $self->{logfile} = "$TYPES{$newtype}{root}/$newtype.log";
>>> }
>>>
>>> return $self->{type};
>>> }
>>>
>>> which changes the type if a new one is supplied and then returns the
>>> current value.
>>
>> I was struggling with this before I left work. In a line like
>> print "Starting with ", $self->[0]->{type},"\n";
>>
>> I got
>>
>> Starting with HASH(x0023408)
>>
>> Not what as I was hoping for. Perhaps I was calling it correctly.
>
> My guess is that you assigned
>
> $self->[0]{type} = $KNOWNTYPES{$type};
>
> or something like that. You have stored a hash reference in a fields that
> should
> be a simple string.
>
>
> Assign the type field in your constructor - 'new'. Then supply a 'get_type'
> method if you think you'll need it.
>
>> Thanx for all the comments. Much appreciated. Peer review rocks!
>
> Keep us informed :)
>
> Rob
There isn't a point there that I don't agree with. Particularly the
point about this being a learning project. I have no problem using OO
modules, or the OO interface of a modules because some authors are
diligent enough to provide both. However I had an encounter with
Catalyst recently and I felt very out of my depth. Hence the desire to
get more familiar with OO.
I think I'll have to take the code home and work on it there as i have
to focus on other things for the next few days. Thanx to all for all
the comments though. There are great help. :-)
Dp.
I'll be back.
--
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
http://learn.perl.org/
