Boolean String Normalizer

Question

How can the normalize() method below be improved? I've extracted the functionality from a project I'm working on into a simple class for the purposes of this question.

The method takes a string input and compares it to a set of 'truthy' and 'falsey' strings, returning true or false respectively if a case insensitive match is found. The truthy and falsey strings are mutable so cannot be hard coded into the method.

If the $nullable property is set to true and no match is found, null is returned. Otherwise false is returned.

I feel as though I may be missing a trick by looping through the array and calling strcasecmp twice for each iteration.

<?php
class BooleanNormalizer
{
 /** @var array */
 public $binaries = [
 'yes' => 'no',
 '1' => '0',
 'on' => 'off',
 'enabled' => 'disabled',
 ];
 /** @var bool */
 public $nullable = false;
 /**
 * @param string $value
 *
 * @return bool|null
 */
 public function normalize($value)
 {
 foreach ($this->binaries as $truthy => $falsey) {
 if (strcasecmp($value, $truthy) === 0) {
 return true;
 }
 if (strcasecmp($value, $falsey) === 0) {
 return false;
 }
 }
 return ($this->nullable) ? null : false;
 }
}

Usage:

$normalizer = new BooleanNormalizer();
$normalizer->nullable = true;
$normalizer->binaries = ['enabled' => 'disabled'];
$normalizer->normalize('enabled'); // returns true
$normalizer->normalize('disabled'); // returns false
$normalizer->normalize('blah'); // returns null

Answer 1

I guess foremost, I question the mutability of this class. Should one really be able to change the value of $binaries?

---

I very much question the optional "nullable" thing here. First of all, name-wise this doesn't seem to make sense as typically when you think of something as "nullable" it means it is a variable, property, field, etc. that is allowed to be set to null. You are not setting anything here, so the terminology seems weird. Perhaps it should be called $returnFalseOnMissingValue or something that more directly ties it to the return behavior of the normalize() method.

I would however get rid of it, as it will become a source of fragility to calling code, as in some cases (including default case) your calling code would not be able to distinguish a false "hit" result from a false "miss" result. Let the calling code deal with converting null to false if it needs to send false up the call stack. Don't obfuscate the result right here in this class which is designed to own this determination.

---

Why use strcasecmp()? This seems an odd choice vs. just casting $value to lowercase for direct comparison. i.e.

$value = strtolower($value);
if ($value === $truthy) return true;
if ($value === $falsey) return false;
return null;

---

I also don't understand the lookup mechanism and comparison mechanisms here. It seems obscure upon first read to have key/value positioning determine how the string is mapped to true/false. I think I would be more explicit like:

protected $mappings = [
 'yes' => true,
 'no' => false,
 '1' => true,
 '0' => false,
 'on' => true,
 'off' => false;
 'enabled' => true,
 'disabled' => false
];

And have your comparison method simply do something like:

public function normalize($value) {
 $value = strtolower($value);
 if(!array_key_exists($value, $this->mappings) return null;
 return $this->mappings[$value];
}

If you truly need to keep the concept of true/value pairs for some reason not shown in this code, then perhaps a structure like:

protected $mappings = [
 'yes' => ['boolean' => true, 'inverse' => 'false'],
 'no' => ['boolean' => false, 'inverse' => 'true'],
 ...
];

This prevents you from having to iterate through the $binaries values every time you want to do a lookup like you are currently doing. The name $binaries also seems odd here as that typically has a much different meaning when talking about writing code.

---

I would suggest that 'true' and 'false' should be strings in your default mapping of boolean values.

---

Does this class really need a concrete instantiation?

---

Consider adding some parameter validation anytime you have public methods. Ideally this could be a combination of type hinting and/or variable inspection to make sure you are being passed a valid parameter. What happens now you if your method is passed an integer, array, object, empty string, etc.?

• Andy: Thank you for the comprehensive answer. For context, this functionality was extracted from a CSV file reader library and is used to format/normalize an individual cell's contents. Because it's library code, users need to be able to specify the different binary pairs that could appear in the file being imported. The nullable property exists because a cell (field) value could well be null if it is not true/false. 'true' and 'false' are included in my default mappings, I just didn't include them in this sample.

Answer 2

A value => boolean map could be used instead of a truthy => falsey map so that the foreach loop and calls to strcasecmp() can be replaced with one isset() check.

You could add a property to the class for case sensitivity and lower case the values when it is false:

<?php
class BooleanNormalizer
{
 /** @var array */
 public static $defaultBinaries = [
 'yes' => 'no',
 '1' => '0',
 'on' => 'off',
 'enabled' => 'disabled',
 ];
 /** @var bool */
 public $nullable;
 /** @var bool */
 public $caseSensitive;
 /** @var array */
 private $values = [];
 public function __construct($nullable = false, $caseSensitive = false, $binaries = null)
 {
 $this->nullable = $nullable;
 $this->caseSensitive = $caseSensitive;
 if (func_num_args() === 2) {
 $binaries = static::$defaultBinaries;
 }
 foreach ($binaries as $truthy => $falsey) {
 $this->addBinary($truthy, $falsey);
 }
 }
 /**
 * @param string $truthy
 * @param string $falsey
 */
 public function addBinary($truthy, $falsey)
 {
 if (!$this->caseSensitive) {
 $truthy = mb_strtolower($truthy);
 $falsey = mb_strtolower($falsey);
 }
 $this->values[$truthy] = true;
 $this->values[$falsey] = false;
 }
 /**
 * @param string $value
 *
 * @return bool|null
 */
 public function normalize($value)
 {
 if (!$this->caseSensitive) {
 $value = mb_strtolower($value);
 }
 if (isset($this->values[$value])) {
 return (bool)$this->values[$value];
 }
 return ($this->nullable) ? null : false;
 }
}

Answer 3

Update: based on your clarified question I have updated my answer

Here's what I whipped up:

• Basically i have created a hash (which is basically a dictionary using c# language, or a set of key values pairs). You can iterate through the key value pairs to get what you want. But I don't have a good idea of what you are actually trying to do - it suggests to me that the code could be refactored to be better from an overall viewpoint, rather than merely focusing on this boolean string normalizer class. i hope this helps. @MikeBrant makes some points worth considering too.

  you can set the hash/dictionary values for example like below:

  @n.binaries = {true => ['enabled', 'yes', 'on', '1'], false => ['disabled', 'no', 'off', '0']}

Then you just iterate through the values and if there's a match, then you return the appropriate key.

Normalizer.rb

class Normalizer
 attr_accessor :binaries
 def normalize(input_value)
 # iterates through the binaries and 
 # returns the appropriate true/false values 
 binaries.each do |boolean_value_key, array|
 array.each do |input|
 if input == input_value 
 return boolean_value_key
 end
 end
 end 
 return nil 
 end
end

Normalizer_tests.rb

# magic_ball.rb
require 'minitest/autorun'
require_relative 'normalizer'
class NormalizerTest < Minitest::Test
 def setup
 @n = Normalizer.new()
 @n.binaries = {true => ['enabled', 'yes', 'on', '1'], false => ['disabled', 'no', 'off', '0']}
 end
 def test_returns_true_for_enabled 
 result = @n.normalize('enabled')
 assert_equal true, result
 end
 def test_returns_false_for_off
 result = @n.normalize('off')
 assert_equal false, result
 end
 def test_returns_nil_for_blah
 result = @n.normalize('blah')
 assert_equal nil, result
 end
end

• Martin R: A Ruby answer for a PHP question?
• BenKoshy: @MartinR good point - but its just syntax: the concept is what i think OP is looking for.
• Andy: Thanks for your answer. I've edited my question to explain what the method actually does. I can't hard code the truthy and falsey values into the method because they are mutable.