Page 1 of 1

Too much code (help with elegant loop) ?

Posted: Thu Jul 08, 2010 6:47 am
by MiniMonty
Hi all,

I've inherited a project that I'm trying to make sense of and it's full of huge code blocks which I
can't help but think could be reduced by using more "elegant" code - just wondering if anyone can help with
this example: (it refers to the first of seven checkboxes representing the days of the week). This code is
repeated seven times on the page (once for each checkbox).

any help at reducing the code by the elegant use of loops much appreciated.

best wishes
Monty

Code: Select all

						<td class="day">
                        <?php 
						if (empty($_POST['datepicker'])) {
							$day_name=date('D'); 
							 $date_spread="".date('m')."/".date('d')."/".date('Y')."";
							 $date_format=explode("/", $date_spread);
							$date_spread="".$date_format[0]."/".$date_format[1]."/".$date_format[2]."";
						} 
						else {
							$date_format=explode("/", $_POST['datepicker']);
							$date_spread="".$date_format[0]."/".$date_format[1]."/".$date_format[2]."";
						} ?>
                            <span><?php 
                            if (!isset($_POST['refresh_s'])) {
								$day1= mktime(0, 0, 0, $date_format[0], $date_format[1], $date_format[2]);
							}
							else {
								$day1= mktime(0, 0, 0, $date_format[0], $date_format[1], $date_format[2]);
								$f="0";
                            	if (date('D', $day1)=="Thu") {
                            		if ($_POST['thu']!="on") { $f=$f+1; 
                            			 if ($_POST['fri']!="on") { $f=$f+1; 
                            			 	if ($_POST['sat']!="on") { $f=$f+1; 
                            			 		if ($_POST['sun']!="on") { $f=$f+1; 
                            			 			if ($_POST['mon']!="on") { $f=$f+1; 
                            			 				if ($_POST['tue']!="on") { $f=$f+1;
                            			 					if ($_POST['wed']!="on") { $f=$f+1;
                            			 					}
                            			 				} 
                            			 			}
                            			 		}
                            			 	}
                            			 }
                            		}
                            	}
                            	if (date('D', $day1)=="Fri") {
                            		if ($_POST['fri']!="on") { $f=$f+1; 
                            			 if ($_POST['sat']!="on") { $f=$f+1; 
                            			 	if ($_POST['sun']!="on") { $f=$f+1; 
                            			 		if ($_POST['mon']!="on") { $f=$f+1; 
                            			 			if ($_POST['tue']!="on") { $f=$f+1; 
                            			 				if ($_POST['wed']!="on") { $f=$f+1;
                            			 					if ($_POST['thu']!="on") { $f=$f+1;
                            			 					}
                            			 				} 
                            			 			}
                            			 		}
                            			 	}
                            			 }
                            		}
                            	}
                            	if (date('D', $day1)=="Sat") {
                            		if ($_POST['sat']!="on") { $f=$f+1; 
                            			 if ($_POST['sun']!="on") { $f=$f+1; 
                            			 	if ($_POST['mon']!="on") { $f=$f+1; 
                            			 		if ($_POST['tue']!="on") { $f=$f+1; 
                            			 			if ($_POST['wed']!="on") { $f=$f+1; 
                            			 				if ($_POST['thu']!="on") { $f=$f+1;
                            			 					if ($_POST['fri']!="on") { $f=$f+1;
                            			 					}
                            			 				} 
                            			 			}
                            			 		}
                            			 	}
                            			 }
                            		}
                            	}
                            	if (date('D', $day1)=="Sun") {
                            		if ($_POST['sun']!="on") { $f=$f+1; 
                            			 if ($_POST['mon']!="on") { $f=$f+1; 
                            			 	if ($_POST['tue']!="on") { $f=$f+1; 
                            			 		if ($_POST['wed']!="on") { $f=$f+1; 
                            			 			if ($_POST['thu']!="on") { $f=$f+1; 
                            			 				if ($_POST['fri']!="on") { $f=$f+1;
                            			 					if ($_POST['sat']!="on") { $f=$f+1;
                            			 					}
                            			 				} 
                            			 			}
                            			 		}
                            			 	}
                            			 }
                            		}
                            	}
                            	if (date('D', $day1)=="Mon") {
                            		if ($_POST['mon']!="on") { $f=$f+1; 
                            			 if ($_POST['tue']!="on") { $f=$f+1; 
                            			 	if ($_POST['wed']!="on") { $f=$f+1; 
                            			 		if ($_POST['thu']!="on") { $f=$f+1; 
                            			 			if ($_POST['fri']!="on") { $f=$f+1; 
                            			 				if ($_POST['sat']!="on") { $f=$f+1;
                            			 					if ($_POST['sun']!="on") { $f=$f+1;
                            			 					}
                            			 				} 
                            			 			}
                            			 		}
                            			 	}
                            			 }
                            		}
                            	}
                            	if (date('D', $day1)=="Tue") {
                            		if ($_POST['tue']!="on") { $f=$f+1; 
                            			 if ($_POST['wed']!="on") { $f=$f+1; 
                            			 	if ($_POST['thu']!="on") { $f=$f+1; 
                            			 		if ($_POST['fri']!="on") { $f=$f+1; 
                            			 			if ($_POST['sat']!="on") { $f=$f+1; 
                            			 				if ($_POST['sun']!="on") { $f=$f+1;
                            			 					if ($_POST['mon']!="on") { $f=$f+1;
                            			 					}
                            			 				} 
                            			 			}
                            			 		}
                            			 	}
                            			 }
                            		}
                            	}
                            	if (date('D', $day1)=="Wed") {
                            		if ($_POST['wed']!="on") { $f=$f+1; 
                            			 if ($_POST['thu']!="on") { $f=$f+1; 
                            			 	if ($_POST['fri']!="on") { $f=$f+1; 
                            			 		if ($_POST['sat']!="on") { $f=$f+1; 
                            			 			if ($_POST['sun']!="on") { $f=$f+1; 
                            			 				if ($_POST['mon']!="on") { $f=$f+1;
                            			 					if ($_POST['tue']!="on") { $f=$f+1;
                            			 					}
                            			 				} 
                            			 			}
                            			 		}
                            			 	}
                            			 }
                            		}
                            	}
                            	$day1= mktime(0, 0, 0, $date_format[0], ($date_format[1]+$f), $date_format[2]);
							}

Re: Too much code (help with elegant loop) ?

Posted: Thu Jul 08, 2010 8:57 am
by Skiddles2010
(it refers to the first of seven checkboxes representing the days of the week)
This is a mess... what exactly is this bit of code trying to do and maybe we can rewrite it from the ground up. It doesn't look too complicated. I'd think that'd be a much better approach than trying to tweak and modify this lazy coder's work...

Re: Too much code (help with elegant loop) ?

Posted: Thu Jul 08, 2010 9:29 am
by Weirdan
Seems it does something like this:

Code: Select all

<?php

/**
 * countConsecutiveUncheckedCheckboxes
 *
 * @param int $startWith
 * @access public
 * @return int
 */
function countConsecutiveUncheckedCheckboxes($startWith, $checkboxes)
{
    $ret = 0;
    $current = new DateTime($startWith);
    for ($i = 0; $i < 8; $i++) {
        $weekday = strtolower($current->format('D'));
        if (isset($checkboxes[$weekday]) && $checkboxes[$weekday] == 'on') {
            break;
        }
        $ret++;
        $current->modify('+1 day');
    }
    return $ret;
}

 $tests = array(
     array('name'   => 'no checked checkboxes return 8',
           'result' => 8,
           'args'   => array('startWith' => '2010-07-08',
                             'checkboxes' => array('sun' => null,
                                                   'mon' => null,
                                                   'tue' => null,
                                                   'wed' => null,
                                                   'thu' => null,
                                                   'fri' => null,
                                                   'sat' => null),),),

     array('name'   => 'all checked checkboxes return 0',
           'result' => 0,
           'args'   => array('startWith' => '2010-07-08',
                             'checkboxes' => array('sun' => 'on',
                                                   'mon' => 'on',
                                                   'tue' => 'on',
                                                   'wed' => 'on',
                                                   'thu' => 'on',
                                                   'fri' => 'on',
                                                   'sat' => 'on'),),),

     array('name'   => 'count stops at first checked checkbox',
           'result' => 2,
           'args'   => array('startWith' => '2010-07-08',
                             'checkboxes' => array('sun' => null,
                                                   'mon' => null,
                                                   'tue' => null,
                                                   'wed' => null,
                                                   'thu' => null,
                                                   'fri' => null,
                                                   'sat' => 'on'),),),

     array('name'   => 'count wraps',
           'result' => 4,
           'args'   => array('startWith' => '2010-07-08',
                             'checkboxes' => array('sun' => null,
                                                   'mon' => 'on',
                                                   'tue' => null,
                                                   'wed' => null,
                                                   'thu' => null,
                                                   'fri' => null,
                                                   'sat' => null),),),
 );

 foreach ($tests as $test) {
     echo $test['name'] . PHP_EOL;
     assert(var_export($test['result'], true)
         . '==' . var_export(countConsecutiveUncheckedCheckboxes($test['args']['startWith'],
                                                                 $test['args']['checkboxes']), true) . ';');
     assert(var_export($test['result'], true)
         . '== countConsecutiveUncheckedCheckboxes('
                 . var_export($test['args']['startWith'], true) . ','
                 . var_export($test['args']['checkboxes'], true)
            . ');');
     echo PHP_EOL;
 }
To use it replace those messy ifs() with

Code: Select all

$f = countConsecutiveUncheckedCheckboxes(date('Y-m-d', $day1), $_POST);