Hi Kevin,

On Thu, May 15, 2014 at 10:10:01AM -0700, Kevin Robert Keegan wrote:

> > @@ -275,7 +275,7 @@ sub default_hop_count
> >  	if (defined($hop_count)){
> >  		::print_log("[Insteon::BaseObject] DEBUG3: Adding hop count of " . $hop_count . " to hop_array of "
> >  			. $self->get_object_name) if $self->debuglevel(3, 'insteon');
> > -		if (!defined(@{$$self{hop_array}})) {
> > +		if ($$self{hop_array} && !(@{$$self{hop_array}})) {
> 
> Oops, no that won't work with strict enabled.  But this does `if (!$$self{hop_array} || !(@{$$self{hop_array}})) {`
> 
> So now if the hash key is undefined OR the value is not an array then we do something.  That should do it.

I have no idea what the above test is meant to accomplish but I have a
few comments, just in case they help to find issues with the new code:

- The original test apparently wanted to check if $self->{hop_array} (an
array reference) existed.

- I *think* the new test before Kevin's comment checks that
$self->{hop_array} is not undef, and that the size of the array is zero
(using an array in scalar context produces the length of the array). Is
this not correct?

- Kevin's proposed change actually changes the logic result of the test.
Perhaps the original test as proposed was incorrect and Kevin's proposed
change fixes it, I don't know. But I do now that A AND B = (NOT A) OR
(NOT B) so if the original test is:

$$self{hop_array} && !(@{$$self{hop_array}})

it can be re-written as follows without changing the logic result:

!$$self{hop_array} || (@{$$self{hop_array}})

But Kevin's proposed change leaves the NOT ('!') in the operand after
the logical OR. Is that intended?

Cheers,

Eloy Paris.-