0

Currently I'm trying to clean up as much code as possible of the website of the company I work at, without like completely rewriting (stuff like removing dumb comments like // declare variables, clean variable naming, consistency etc.). The code is a huge mess and terrible, uses lots of files which gets included and variables get used between files which makes it really annoying.

The problem is, a simple thing as renaming a variable actually can break a lot. I currently have the following problem:

In file a I have a query, looking something like this:

$getProductSql = 'QUERY';
$getProduct = $db->prepare($getProductSql);
// some bind values etc.
$getProduct->execute();
$product = $getProduct->fetch(PDO::FETCH_ASSOC);

Now, after that file is included in the index, file b will get included which contains the following:

$getProductsSql = 'QUERY';
$getProducts = $db->prepare($getProductsSql);
// some bind values etc.
$getProducts->execute();

foreach ($getProducts->fetchAll(PDO::FETCH_ASSOC) as $product) {
    // some code
}

After that file is included, file c will be included which contains the following:

if ($product['COLUMN'] === '1')

In file c, the $product variable from file a should be used but due to how our structure is and file b getting included in between, $product from file a is replaced with the last value of $product from the loop in file b.

Is there any way to solve this without using 2 different variable names or moving code?

Dave
  • 5,108
  • 16
  • 30
  • 40
Joshua Bakker
  • 2,288
  • 3
  • 30
  • 63
  • https://blog.joefallon.net/2015/08/immutable-objects-in-php/ – online Thomas Dec 03 '18 at 15:09
  • Where is the $product in file c being defined? If previously it was pulling in from file b then it sounds like it was an existing bug. If not then how was it being assigned and why was it using the same name? – Jeff Dec 03 '18 at 15:14
  • 3
    This is pretty much exactly why you use *functions*, and why functions explicitly scope variables. See https://stackoverflow.com/a/16959577/476. – deceze Dec 03 '18 at 15:15
  • Also what is the code in the for loop doing? – Jeff Dec 03 '18 at 15:16
  • @Jeff The `$product` in file `a` was something before but I changed it because this made more sense. That same old variable was also used in file `c`. (so lets say `$product` was previously `$thecoolproduct`, file `c` used this `$thecoolproduct`. The code in the foreach adds some HTML to certain variables (keep in mind, this isn't my code and the code is a huge mess made by somebody who didn't know what he was doing) – Joshua Bakker Dec 03 '18 at 15:24
  • In that case I would change the foreach loop to have a variable with a more transient name like $p or something similar for now to get it working I'm assuming you plan on refactoring all of it – Jeff Dec 03 '18 at 15:29
  • I already told the "IT manager" and he said to rewrite it all (which basically has to be done since we're not using a framework and this code is a huge mess which makes it pretty hard to move and refactor) costs too much time, but eventually we'd have to change it since this is terrible. I don't like variable names like `$p` or something but as @deceze said I'll try to solve it with a function first. – Joshua Bakker Dec 03 '18 at 15:33

1 Answers1

1

As deceze says: there isn't really ... but you could do this:

Leave file a as is.

Change file b like this:

$getProductsSql = 'QUERY';
$getProducts = $db->prepare($getProductsSql);
// some bind values etc.
$getProducts->execute();

$p[] = $product;

foreach ($getProducts->fetchAll(PDO::FETCH_ASSOC) as $product) {
    // some code
    $p[] = $product
}
$product = $p;

And file c like this:

if ($product[0]['COLUMN'] === '1')

It is a bit confusing and not really nice. And yes, that is why you should rather use a function ...

Alternatively: If you don't need the product array from file b any longer in file c you could also simply do this:

file b:

$getProductsSql = 'QUERY';
$getProducts = $db->prepare($getProductsSql);
// some bind values etc.
$getProducts->execute();

$p = $product;

foreach ($getProducts->fetchAll(PDO::FETCH_ASSOC) as $product) {
    // some code
}
$product = $p;

In that case: no need to change file c

rf1234
  • 1,510
  • 12
  • 13
  • I think it's cleaner to use functions so I'm gonna look into the code and see if I can make a function around the loop. Else I'll use this method which I'll try to avoid simply due to the fact the extra variable necessary kind of bugs me. Thanks anyways – Joshua Bakker Dec 03 '18 at 15:37