0

I have a great deal of 'dirty' data pulled from an API request.

Before database insertion, I clean out a lot of line feeds and regex out values such as height and width from item descriptions.

This is my first large project as a software engineer, and I would like to leave something coherent for future developers.

Here is one such example:

private function lineItemHeight($invoiceLineItem)
{
    if(preg_match("/(\d+mm W)|(\d+mm H)/", $invoiceLineItem, $matches) == 1)
    {
        $height = $matches[1];
        return preg_replace("/[^0-9,.]/", "", $height);
    }
    else
    {
        return 0;
    }
}

And another:

private function contactName()
{
    // Removes appended (email@address.sample) from contact name
    return preg_replace('/\([^)]*\)/', "", $this->contactName);
}

Would I be better off having a SanatiseInvoiceData class to parse all the strings before they are set, or is my current practice acceptable.

I am looking for industry or expected standard. What I have works -not knowing how well it stands up to scrutiny makes me uncomfortable.

2

Parsing records is a bread and butter programming problem, and there are at least a dozen ways to do it.

The way that has always seemed the clearest to me, when maintaining very old code, is when the programmer takes the trouble to define a class that represents a record or row itself (e.g. a Record class, not a RecordParser class). That means a stateful class. Might look like this:

class SanatisedInvoiceLineItem {
    private $text;

    function __construct($originalText) {
        $this->text = $originalText;
    }

   function getHeight()
   {
       if(preg_match("/(\d+mm W)|(\d+mm H)/", $this->text, $matches) !== 1)
       {
           return 0;
       }
       $height = $matches[1];
       return preg_replace("/[^0-9,.]/", "", $height);
    }  
}

To use it:

$line = new SanitisedInvoiceLineItem($textFromApi);
$height = $line=>getHeight();

Couple notes:

  1. The class naturally provides a namespace, so your function names can get shorter (getHeight instead of lineItemHeight).

  2. You can validate the entire line item in the constructor, if you want. This may be more suitable than validating each field separately.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.