0

I'm new to programming and in particular new to PHP. I often find that DRY is directly opposed to the principle of keeping variables local in PHP functions. Sometimes I think that it's easier to violate DRY to be able to readily access large arrays than it is to pass these arrays or their relevant subcomponents as parameters in functions.

Does anyone have thoughts on this or examples of code that finds another way? Here's an example of the kind of code I sometimes write because my arrays are large and I don't want to copy/pass them or have to write out all the chunks I might use in a function, hence deciding to violate DRY and not use a function:

if($array1['status'][$i]==1)
{
    for($j = 1; $j < 3; $j ++)
    {
        $j = 1;
        $dist = $array2['dist'][$j];
        $id =  $array2['id'][$j];
        $email =  $array2['email'][$j];
        send_offer_email($array1['appt_id'][$i], $array1['fulladd'][$i], $dist, $id, $email);
        $j = 2;
        $dist = $array2['dist'][$j];
        $id =  $array2['id'][$j];
        $email =  $array2['email'][$j];
        send_offer_email($array1['appt_id'][$i], $array1['fulladd'][$i], $dist, $id, $email);
    }
}
elseif($array1['status'][$i]==2)
{
    for($j = 3; $j < 7; $j ++)
    {
        $j = 1;
        $dist = $array2['dist'][$j];
        $id =  $array2['id'][$j];
        $email =  $array2['email'][$j];
        send_offer_email($array1['appt_id'][$i], $array1['fulladd'][$i], $dist, $id, $email);
        $j = 2;
        $dist = $array2['dist'][$j];
        $id =  $array2['id'][$j];
        $email =  $array2['email'][$j];
        send_offer_email($array1['appt_id'][$i], $array1['fulladd'][$i], $dist, $id, $email);
    }
}

Clearly I'm a noob, but I'd love to hear what's wrong with doing this vs having to write out all the crap I'd need to reduce each of these if branches to something like:

$j = 1
function(blah blah, $j)

In particular the blah blah in the function above would be really long and itself violate DRY.

Thanks!

  • take a look at switch statements for one thing – HotelCalifornia Apr 9 '15 at 22:44
  • What exactly is this code trying to accomplish? – Robert Harvey Apr 9 '15 at 22:52
  • Why are you modifying the loop control variable in the loop? – Steven Burnap Apr 9 '15 at 23:10
  • In any case, your repeated code uses two array references and two index variables. That's hardly much for an argument list. If you want to know why DRY is important, imagine how easy it would be to accidently type $i instead of $j just once. – Steven Burnap Apr 9 '15 at 23:13
  • 1
    The other thing that strikes me is that your data structures are poor. If you organize it like an array of records ($array1[$1]['appt_id']) then you can create functions that just take $array1[$1] and everything is a billion times cleaner. – Steven Burnap Apr 9 '15 at 23:16
1

Your code is very difficult to read and it appears your data isn't structured well for what it represents and how it's used. I suspect if you address these issues then the solution to your reusability problem will become clear.

It isn't completely obvious what your code does, but take this example loosely based on what I think your code is doing:

class Applicant {

    public $id;
    public $address;
    public $email;
    public $status;

    public function sendDecision() {
        if($this->status == 1) {
            $this->sendOffer();
        } else {
            $this->sendRejection();
        }
    }

    public function sendOffer() {
        // Do something with $this->email
    }

    public function sendRejection() {
        // Do something with $this->email
    }
}

$applications = array(); // Populate this with an array of Applicant instances

foreach($applications as $applicant) {
    $applicant->sendDecision();
}

That keeps you from creating a haze of array indexes which are easy to mix up, keeps related logic and data grouped together, and allows you to do something different depending on the status without duplicating code.

  • thanks for this. I suppose one of my gripes is I'm just retrieving data from a database to prepare an email, and then I won't need the data anymore. For this reason, class methods to me seem like more trouble than they're worth, but I agree my code is quite ugly, and yours is quite readable. Is it fair to say it's always worth writing a class? – sunny Apr 10 '15 at 13:27

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.