Table of Contents
Five classes. Same trait. Same boolean helper method. Every single one had the condition inverted. And nobody noticed for over a year.
The Setup
A reporting module had several column classes that all shared a trait:
trait ChecksEligibility
{
private function isEligible(Order $order): bool
{
$exemptTypes = config('reports.exempt_types', []);
return !in_array($order->type, $exemptTypes);
}
}
Simple enough. Returns true for most orders, false for exempt ones. Five different report column classes used this trait to decide whether to apply a calculation:
class TotalBeforeAdjustment extends ReportColumn
{
use ChecksEligibility;
public function getValue(Order $order): float
{
if (!$this->isEligible($order)) {
return $order->total / 1.09; // Remove the 9% adjustment
}
return $order->total; // No adjustment needed
}
}
Spot the bug?
The Inversion
The ! negation is backwards. For eligible orders (the common case), it should apply the calculation. For exempt orders, it should skip it. But the ! flips the logic — exempt orders get calculated, eligible ones don’t.
Every class using this trait had the same inverted condition. Somewhere, the first developer misread what isEligible returned, and everyone else copy-pasted the pattern.
How to Find It: Empirical Debugging
Don’t trust your reading of the code. Run it.
// In artisan tinker
$order = Order::find(12345); // Known eligible order
$column = new TotalBeforeAdjustment();
$column->getValue($order);
// Expected: 100.00 (no adjustment)
// Got: 91.74 (adjustment applied!)
$exemptOrder = Order::find(67890); // Known exempt order
$column->getValue($exemptOrder);
// Expected: 91.74 (adjustment applied)
// Got: 100.00 (no adjustment!)
Two test cases. One eligible, one exempt. The outputs are swapped. Bug confirmed in under 30 seconds.
The Fix
// Before (wrong)
if (!$this->isEligible($order)) {
// After (correct)
if ($this->isEligible($order)) {
One character removed. Repeated across 5 classes.
Why It Hid for So Long
Three reasons:
- Low-traffic feature. The report was used monthly by a handful of people. Nobody cross-checked the numbers against source data.
- Method naming confusion. “isEligible” could reasonably mean either “should we apply the calculation” or “is this order in the standard category”. The name was technically correct but invited misinterpretation.
- Copy-paste propagation. Once the first class got it wrong, every subsequent class copied the same pattern. Consistency made the bug look intentional.
Prevention
When you share boolean helpers via traits:
- Name the method for what the CALLER does with it.
shouldApplyAdjustment()is harder to invert thanisEligible(). - Add a unit test with both cases. One eligible, one exempt. Takes 2 minutes, catches inversions immediately.
- Be suspicious of
!before trait methods. If every consumer negates the return value, either the method name is misleading or the logic is inverted.
The takeaway: Boolean bugs don’t crash your app — they silently produce wrong data. The only reliable way to catch them is to test with known inputs and verify the outputs match your expectations. Read the code, then run the code.
Leave a Reply