Ticket #397 (closed cage: fixed)

Opened 13 years ago

Last modified 13 years ago

Double negatives need inverting in most uses of Parrot_str_not_equal

Reported by: Util Owned by:
Priority: minor Milestone:
Component: core Version: trunk
Severity: low Keywords: refactor Parrot_str_equal Parrot_str_not_equal
Cc: Language:
Patch status: Platform: all

Description

History:
Due to its strcmp(3) parentage, Parrot_str_equal (prior to r36336) returned true when not equal, and false when equal. In r36336 and r36338, the return value for Parrot_str_equal was inverted, to match the intuitive meaning of the function's name. The new function Parrot_str_not_equal was created as a drop-in replacement for the old API of Parrot_str_equal, and s/Parrot_str_equal/Parrot_str_not_equal/ was done in the relevant files.

This change cleared up the function's bad API. It did not address the refactoring needed to make client code read as if the improved API had always existed.

Problem:
This pattern occurs over 150 times in our code:

if (Parrot_str_not_equal(foo,bar) == 0) {...}

This is a violation of PDD07, in the section "Avoid double negatives".

Solution:
These phrases, and their equivalents:

    (Parrot_str_not_equal(foo,bar) == 0)
    (Parrot_str_not_equal(foo,bar) != 0)

should be refactored to simple booleans:

    Parrot_str_equal(foo,bar)
    Parrot_str_not_equal(foo,bar)

Change History

Changed 13 years ago by Util

  • status changed from new to closed
  • resolution set to fixed

Resolved in r37099.

Note: See TracTickets for help on using tickets.