«

Hi All,

I'm fairly new to perl, and have already fixed this problem, but I
want to understand why it didn't work before.

This is the original, non-working code:

@o_list = ();
foreach my $o ( @{$stuff->{thingys}} ) {

# Exclude if no match
$match = 'n';
foreach $thingy (@test_thingys) {

if ($o->{thingy} == $thingy) {
$match = 'y';
last;
}
}
next if ($match != 'y');

push @o_list, $o;
}

Basically, its running through a list of "thingys", checking to see if
the item matches any of a list of test "thingys." Before checking the list,
I set $match to 'n'. If I find a match, I set it to 'y' and dump out of the
loop. At least, that was the intent. What really happened was that "next
if" line never ran, and all items got pushed onto "o_list." I did some
checking, and found that $match equaled 'n'. The line simply wasn't ever
running.

The working snippet...

@o_list = ();
foreach my $o ( @{$stuff->{thingys}} ) {

# Exclude if no match
$match = 0;
foreach $thingy (@test_thingys) {

if ($o->{thingy} == $thingy) {
$match = 1;
last;
}
}
next if (!match);

push @o_list, $o;
}

The only change was the "flag" values. Instead of 'y' and 'n', it's
now 0 and 1.

Can anyone tell me what about the 'n' and 'y' messed up the logic?

Thanks for your help,
Max

>>>>> "MM" == Max Moor <maxmoor@remove_hotmail.com> writes:

MM> next if ($match != 'y');

that does a numeric comparison. 'n' is 0 and so is 'y' so that will
never be true. use eq for string compares. if you had enabled warnings
perl would have told you this. also you aren't using strict which helps
too. ask perl for all the help it can give you.

now the bigger picture is you shouldn't be using boolean flags like that
in general. proper use of next/last will eliminate the need for them. i
don't have the time to review the logic but others may do so.

uri

--
Uri Guttman ------ -------- http://www.sysarch.com --
----- Perl Code Review , Architecture, Development, Training, Support ------
--------- Free Perl Training --- http://perlhunter.com/college.html ---------
--------- Gourmet Hot Cocoa Mix ---- http://bestfriendscocoa.com ---------

Uri Guttman <> wrote in
news::

>>>>>> "MM" == Max Moor <maxmoor@remove_hotmail.com> writes:
>
> MM> next if ($match != 'y');
>
> that does a numeric comparison. 'n' is 0 and so is 'y' so that will
> never be true. use eq for string compares. if you had enabled warnings
> perl would have told you this. also you aren't using strict which helps
> too. ask perl for all the help it can give you.
>
> now the bigger picture is you shouldn't be using boolean flags like that
> in general. proper use of next/last will eliminate the need for them. i
> don't have the time to review the logic but others may do so.
>
> uri
>

Hi uri,

Thanks for the answer. Makes sense.

I also agree with you about flags, but there's still alot of ugly in
this code. It's really just an eval block inside a larger perl script that
we users on a particular site are allowed access to. We can use it to
price check items we have for sale.

I'm a C programmer from way back, but haven't done perl before. I
started with the code as generated by an auto-writer they provide.
Hopefully I'll get to know my way around perl well enough that some things
like bad logic will get cleaned up.

I'll see what I can do to add warnings and strict-ness. I'm not sure
how that might affect the larger script I don't have say over.

Anyway, thanks again for the knowledge. I appreciate the help.

- Max

On 15 Jul 2009 19:24:27 GMT, Max Moor <maxmoor@remove_hotmail.com> wrote:

>Hi All,
>
> I'm fairly new to perl, and have already fixed this problem, but I
>want to understand why it didn't work before.
>
> This is the original, non-working code:
>
>@o_list = ();
>foreach my $o ( @{$stuff->{thingys}} ) {
>
> # Exclude if no match
> $match = 'n';
> foreach $thingy (@test_thingys) {
>
> if ($o->{thingy} == $thingy) {

if .. == .. or .. eq ..
> $match = 'y';
> last;
> }
> }
> next if ($match != 'y');

if .. != .. or .. ne ..
>
> push @o_list, $o;
>}

Take your pick. Not both.

-sln

In article <>, wrote:
>On 15 Jul 2009 19:24:27 GMT, Max Moor <maxmoor@remove_hotmail.com> wrote:
>
>>Hi All,
>>
>> I'm fairly new to perl, and have already fixed this problem, but I
>>want to understand why it didn't work before.
>>
>> This is the original, non-working code:
>>
>>@o_list = ();
>>foreach my $o ( @{$stuff->{thingys}} ) {
>>
>> # Exclude if no match
>> $match = 'n';
>> foreach $thingy (@test_thingys) {
>>
>> if ($o->{thingy} == $thingy) {
>
>if .. == .. or .. eq ..

depending on the type of the two operands.

>> $match = 'y';
>> last;
>> }
>> }
>> next if ($match != 'y');
>
>if .. != .. or .. ne ..

Wrong. ne is necessary for string comparisons.
>>
>> push @o_list, $o;
>>}
>
>Take your pick. Not both.

Nonsense. $match, being assigned values of either 'n' or 'y', clearly requires
the string inequality operator ne. He can't "take [his] pick" there.

Likewise, he can't pick and choose in the other comparison either. If the
operands are numeric, he *must* use ==; if they are strings, he *must* use eq.

On Wed, 15 Jul 2009 21:50:44 GMT, (Doug Miller) wrote:

>In article <>, wrote:
>>On 15 Jul 2009 19:24:27 GMT, Max Moor <maxmoor@remove_hotmail.com> wrote:
>>
>>>Hi All,
>>>
>>> I'm fairly new to perl, and have already fixed this problem, but I
>>>want to understand why it didn't work before.
>>>
>>> This is the original, non-working code:
>>>
>>>@o_list = ();
>>>foreach my $o ( @{$stuff->{thingys}} ) {
>>>
>>> # Exclude if no match
>>> $match = 'n';
>>> foreach $thingy (@test_thingys) {
>>>
>>> if ($o->{thingy} == $thingy) {
>>
>>if .. == .. or .. eq ..
>
>depending on the type of the two operands.
>
>>> $match = 'y';
>>> last;
>>> }
>>> }
>>> next if ($match != 'y');
>>
>>if .. != .. or .. ne ..
>
>Wrong. ne is necessary for string comparisons.
>>>
>>> push @o_list, $o;
>>>}
>>
>>Take your pick. Not both.
>
>Nonsense. $match, being assigned values of either 'n' or 'y', clearly requires
>the string inequality operator ne. He can't "take [his] pick" there.
>
>Likewise, he can't pick and choose in the other comparison either. If the
>operands are numeric, he *must* use ==; if they are strings, he *must* use eq.
^^^^^^^^^^^^^^^^^^^^^^^^^
Nonsense, 2 == ... and 2 eq ...

-sln

On Wed, 15 Jul 2009 14:59:39 -0700, wrote:

>On Wed, 15 Jul 2009 21:50:44 GMT, (Doug Miller) wrote:
>
>>In article <>, wrote:
>>>On 15 Jul 2009 19:24:27 GMT, Max Moor <maxmoor@remove_hotmail.com> wrote:
>>>
>>>>Hi All,
>>>>
>>>> I'm fairly new to perl, and have already fixed this problem, but I
>>>>want to understand why it didn't work before.
>>>>
>>>> This is the original, non-working code:
>>>>
>>>>@o_list = ();
>>>>foreach my $o ( @{$stuff->{thingys}} ) {
>>>>
>>>> # Exclude if no match
>>>> $match = 'n';
>>>> foreach $thingy (@test_thingys) {
>>>>
>>>> if ($o->{thingy} == $thingy) {
>>>
>>>if .. == .. or .. eq ..
>>
>>depending on the type of the two operands.
>>
>>>> $match = 'y';
>>>> last;
>>>> }
>>>> }
>>>> next if ($match != 'y');
>>>
>>>if .. != .. or .. ne ..
>>
>>Wrong. ne is necessary for string comparisons.
>>>>
>>>> push @o_list, $o;
>>>>}
>>>
>>>Take your pick. Not both.
>>
>>Nonsense. $match, being assigned values of either 'n' or 'y', clearly requires
>>the string inequality operator ne. He can't "take [his] pick" there.
>>
>>Likewise, he can't pick and choose in the other comparison either. If the
>>operands are numeric, he *must* use ==; if they are strings, he *must* use eq.
> ^^^^^^^^^^^^^^^^^^^^^^^^^
>Nonsense, 2 == ... and 2 eq ...
>
>-sln

There should actually be a 2 e= ... operator, no need for conversion (if necessary).

-sln

On Wed, 15 Jul 2009 21:50:44 GMT, (Doug Miller) wrote:

>In article <>, wrote:
>>On 15 Jul 2009 19:24:27 GMT, Max Moor <maxmoor@remove_hotmail.com> wrote:
>>
>>>Hi All,
>>>
>>> I'm fairly new to perl, and have already fixed this problem, but I
>>>want to understand why it didn't work before.
>>>
>>> This is the original, non-working code:
>>>
>>>@o_list = ();
>>>foreach my $o ( @{$stuff->{thingys}} ) {
>>>
>>> # Exclude if no match
>>> $match = 'n';
>>> foreach $thingy (@test_thingys) {
>>>
>>> if ($o->{thingy} == $thingy) {
>>
>>if .. == .. or .. eq ..
>
>depending on the type of the two operands.
>
>>> $match = 'y';
>>> last;
>>> }
>>> }
>>> next if ($match != 'y');
>>
>>if .. != .. or .. ne ..
>
>Wrong. ne is necessary for string comparisons.
>>>
>>> push @o_list, $o;
>>>}
>>
>>Take your pick. Not both.
>
>Nonsense. $match, being assigned values of either 'n' or 'y', clearly requires
>the string inequality operator ne. He can't "take [his] pick" there.
>
>Likewise, he can't pick and choose in the other comparison either. If the
>operands are numeric, he *must* use ==; if they are strings, he *must* use eq.

But, match != 'y' is valid, isin't it? Isin't that valid???

-sln

(Doug Miller) wrote:
>Likewise, he can't pick and choose in the other comparison either. If the
>operands are numeric, he *must* use ==; if they are strings, he *must* use eq.

Actually you can mix and match. But you better know exactly what you are
doing.

There is nothing stopping you from comparing the string values of two
numbers and actually of the top my head I can't think of a scenario,
where you would get an unexpected surprise.

And you can also compare the numerical values of two strings. However in
this case you really better know what those numerical values are going
to be because most of the time it's probably not what you would expect.

jue

On Wed, 15 Jul 2009 16:11:33 -0700, Jürgen Exner <> wrote:

> (Doug Miller) wrote:
>>Likewise, he can't pick and choose in the other comparison either. If the
>>operands are numeric, he *must* use ==; if they are strings, he *must* use eq.
>
>Actually you can mix and match. But you better know exactly what you are
>doing.
>
>There is nothing stopping you from comparing the string values of two
>numbers and actually of the top my head I can't think of a scenario,
>where you would get an unexpected surprise.
>
>And you can also compare the numerical values of two strings. However in
>this case you really better know what those numerical values are going
>to be because most of the time it's probably not what you would expect.
>
>jue

Very well said.

-sln