Look! A horrible piece of code in a horrible language in a horrible frame for a sickeningly twee ceremony that should have been made obsolete along with the Inquisition!
Let's review it.
Here's the code, with line numbers.
01 <? 02 function do_wed() { 03 if ($objections != true) { 04 function do_vow() { 05 $vow = 1; 06 do { 07 if ($richer === 1 08 && $poorer === 1 09 && $sickness === 1 10 && $health === 1) { 11 function have_hold($a,$b) { 12 ini_set('session.gc_maxlifetime','forever'); 13 } 14 have_hold('husband','wife'); 15 define('friend', true); 16 define('partner', true); 17 define('faithful', true); 18 if ($i = 'do') { 19 $f = 'finger'; 20 $r = 'ring; 21 $f = $f + $r; 22 } 23 } 24 $vow = $vow + 1; 25 } while ($vow != 2); 26 } 27 do_vow(); 28 $register = array_fill($details); 29 print_r($register) 30 return $kiss; 31 } 32 } 33 do_wed(); 34 ?>
Let's go!
line 1
We use long tags here. <?php
line 3
Undefined variable $objections
.
$objections != true
better written !$objections
. But this is not what you meant; you meant count($objections) == 0
, since it will be an array of them
line 4
Don't define functions inside other functions.
lines 6, 25
You know how many vows you want. Use a for loop. Better, use an array of vows and populate it with two Vow objects, which represent the conditions each person agrees to. This means you can marry more than 2 people. The do_wed()
function should take the people to wed as arguments. Use func_get_args()
to loop over all of them, or (...$parties)
in the next version of PHP.
Useless loop anyway. do_vow()
should be called twice with the person currently vowing.
"Twice" is a western concept. This code is not internationalised.
lines 7-10
Undefined variables. None of these equals 1. It is unlikely that all four of these things would equal 1 at the same time. You want to test the party's agreement to these concepts, not the value of these variables. You need Person objects.
line 11
A function in a function in a function? This function takes two parameters and uses neither. Get rid of them.
line 12
This ini parameter takes an integer. 'forever'
is not an integer.
line 13
This closing brace does not line up with the function definition on line 13. It does line up with the if on line 7, which implies you've forgotten to close the function, but scrutiny shows that you've misaligned the brace.
line 14
have_hold
does not take any parameters any more.
This is exclusivist. Not all marriages are between a husband and a wife. These should be parameters to do_wed()
.
This function is run twice, both times with the same parameters. It should swap over for the second iteration.
line 16
'partner'
is presumably the person we are not currently dealing with.
line 17
'faithful'
is not a boolean value and should be configured per app. It needs to be a data structure containing parameters of faithfulness, i.e. boundaries.
line 18
This is always true. Remove this condition. $i
is never used, so remove the assignment too.
lines 19, 20
Useless variables. Either accept them as parameters or use the literal strings directly.
line 21
If you'd not used these useless variables you'd realise you're trying to numerically add strings. .
is the concatenation operator. What is a 'fingerring'?
$f
is discarded. Just omit this entire block.
line 22
What is this supposed to line up with?
line 23
This closes the if
that looks like it is closed on line 13. But it does not line up with it.
line 24
Better written $vow++
, but we've replaced this with an array of Vow objects containing agreement parameters, so don't do this any more.
line 25
The only reason this would be a while loop is if you're just going to keep asking until both (all) parties agree. This is not how one should enter into a marriage.
line 26
This closes do_vow()
but does not line up with it.
line 27
This is what should be run n times, once per party in the agreement.
line 28
array_fill
takes three parameters. Register should be an object.
line 29
Syntax error - missing semicolon.
print_r
is not the best thing to use here. Serialise this properly, perhaps with JSON so it can be consumed by an API or HTML so it can be styled and displayed properly.
line 30
Undefined variable $kiss
. Kiss is a verb and should be a function.
lines 31, 32
These braces should line up with what they close.
line 33
Don't run a function when it is defined - that's not how you create a library.
This function could at least be parameterised with the names of the people being married. Isn't Etsy about crafts and hence personalisation?
No comments:
Post a Comment