Меню
Что нового?

Прошу оценить функцию

Bartman

Новичок
Прошу проревьювить функцию

Приветствую всех!
Передо мной стояла задача: взять временную метку записи из таблицы MySQL, разбить её "на составляющие" и сгенерить соот-щие селекты. Для реализации поставленой задачи я написал функцию, которая отвечает непосредствнно за это. Я хотел бы обратиться к посетителям форума с просьбой проревьювить мою ф-ю на предмет универсальности и оптимальности кода. В текущем представлении ф-я отлажена и полностью работоспособна. Принимаются любые замечания и предложения. Полный код ф-ии привожу ниже.
Заранее благорарю!


PHP: 
$months = array("Январь","Февраль","Март","Апрель","Май","Июнь","Июль","Август","Сентябрь","Октябрь","Ноябрь","Декабрь"); 

function BuildDateSelection ($timestamp = -1, $fieldname, $yearstart, $yearend) { 

global $months;

            if ($timestamp == -1) {
            $timestamp = time();
        }

    // Получаем дату и вермя в ассоциативном массиве
   $today = getdate($timestamp);
			
   $day     = $today["mday"];
   $month   = $today["mon"];
   $year    = $today["year"];
   $hours   = $today["hours"];
   $minutes = $today["minutes"]; 
			
   // Формируем селект дней с позиционированием на текущем дне
   $string = "\n<select name=".$fieldname."_day>";

   for ($x = 1; $x < 32; $x++) {
     $string .= ($x == $day) ? "\n<option value=". $x ." selected>". $x. "</option>" : "\n<option value=". $x .">". $x. "</option>";
   }
   $string .= "\n</select>";

    // Формируем селект месяцев с позиционированием на текущем месяце
    $string .= "\n<select name=".$fieldname."_month>";
	
	for ($x = 1; $x <= 12; $x++) {
	 $string .= ($x == $month) ? "\n<option value=". $x ." selected>". $months[$x-1]. "</option>" : "\n<option value=". $x .">". $months[$x-1]. "</option>";
	}
	$string .= "\n</select>";
	
	// Формируем селект лет с позиционированием на текущем годе
	$string .= "\n<select name=".$fieldname."_year>";
	
	for ($x = $yearstart; $x <= $yearend; $x++) {
	 $string .= ($x == $year) ? "\n<option value=". $x ." selected>". $x. "</option>" : "\n<option value=". $x .">". $x. "</option>";
	}
	$string .= "\n</select>";
	
	// Формируем селект часов с позиционированием на текущем часе
	$string .= "\n<select name=".$fieldname."_hour>";
	
	for ($x = 1; $x < 25; $x++) {
	 $string .= ($x == $hours) ? "\n<option value=". $x ." selected>". $x. "</option>" : "\n<option value=". $x .">". $x. "</option>";
	}
	$string .= "\n</select>";
	
	// Формируем селект минут с позиционированием на текущей минуте
	$string .= "\n<select name=".$fieldname."_minute>";
	
	for ($x = 1; $x <= 60; $x++) {
	 $string .= ($x == $minutes) ? "\n<option value=". $x ." selected>". $x. "</option>" : "\n<option value=". $x .">". $x. "</option>";
	}
	$string .= "\n</select>";
	
	return $string;
} 


	// Пример запроса к MySQL чтобы получить временный штамп в формате UNIX
	$query = "SELECT * , UNIX_TIMESTAMP(data_post) AS time FROM test";
	
	// Пример вызова ф-ии
	BuildDateSelection ($row["time"], "example", 1999, 2005);
 

Bartman

Новичок
RomikChef, я это учёл. В качестве параметра в ф-ю передаётся $timestamp. Я задаю значение этой переменной по умолчанию равно -1. А потом, уже в самой ф-ии я проверяю:

if ($timestamp == -1) {
$timestamp = time();
}
 

RomikChef

Guest
да вижу-вижу уже.
а как ты вызываешь эту функцию, когда у тебя нет поля даты?
когда у тебя срабатывает знаение по умолчанию?
пример вызова?
 

RomikChef

Guest
а зачем ты тогда дефолтное значение при объявлении функции указываешь? :)))))
 

Bartman

Новичок
ну да, получается, что можно при объявлении ф-ии не указывать дефолное значение...
тогда получается так:

PHP: 
function BuildDateSelection ($timestamp, $fieldname, $yearstart, $yearend) {
 

Tronyх

Новичок
Bartman просто если ты делаешь для какого-то аргумента значение по умолчанию (у тебя это -1), этот аргумент ставится САМЫМ ПОСЛЕДНИМ, потому что если его поставить слева(как у тебя) тебе всё равно прийдётся его вводить.
 

Bartman

Новичок
Tronyх, спасибо за подсказку. Я об этом не знал.
Действительтно, если ф-ю объявить как:

PHP: 
function BuildDateSelection ($fieldname, $yearstart, $yearend, $timestamp = -1) {
то вызывать можно так:
BuildDateSelection ("example", 1999, 2005);
 

RomikChef

Guest
лично я бы и бы диапазон дат сделал бы умолченым
к примеру

function BuildDateSelection ($fieldname, $timestamp=-1,$years="") {

if (!$years) {
$yearstart=date("Y")-2;
$yearend=date("Y")+2;
} else list ($yearstart, $yearend)=explode("-",$years);
 

Bartman

Новичок
RomikChef, не совсем понял, в каком формате тогда должна передаваться переменная $years ? Строка?
 

RomikChef

Guest
ну да. "1999-2005"
ну можно их обе сделать числами с умолчаниями.
это уже все изыски, украшения
 

Bartman

Новичок
ага, понял, RomikChef, спасибо... вот такой еще вопрос: насколько оптимальна сделана, например, генерация списка с выделением текущего, например, дня:

$string .= ($x == $day) ? "\\n<option value=". $x ." selected>". $x. "</option>" : "\\n<option value=". $x .">". $x. "</option>";

можно ли как-то по-красивей это сделать или по оптимальности кода этого вполне достаточно ?
 

RomikChef

Guest
ну, вроде, нормально...

ну, если бы не выводить месяц словом, то легко можно было бы сделать массивчик вида
$a['days']['start']=1;
$a['days']['end']=31;
$a['minutes']['start']=00;
и так далее.
и в цикле по нему бежаться :)
впрочем, и массив так можно - одно условие поставить :)
 

RomikChef

Guest
точнее, первый ключ в нем сделать совпадающим с именами ключей в getdate
и проверять
if ($a[$key][$i]==$today[$key]) echo "selected";
 

RomikChef

Guest
понял идею?
просто у меня срабатывает рефлекс, когда я вижу больше двух подобных операций :)

покажешь, что получилось?

кстати, тогда можно будет сделать универсальное поле ввода, и передавать еще один параметр - маску, какие поля выводить.
напрмер $mask="md"
и тогда будет запрошео только месяц и день
 

RomikChef

Guest
ну ё!
у тебя идут подраж 5 циклов фор.
в которых меняется только начальное значение и конечное.
их надо загнать в один общий цикл!

блин, проще написать уже, чем объяснить.
 

RomikChef

Guest
PHP: 
$src["mday"]['start']=1; 
$src["mday"]['end']=31; 
$src["mon"]['start']=1; 
$src["mon"]['end']=12; 
$src["year"]['start']=$yearstart; 
$src["year"]['end']=$yearend; 
$src["hours"]['start']=0; 
$src["hours"]['end']=23; 
$src["minutes"]['start']=0; 
$src["minutes"]['end']=59; 
foreach ($src as $part => $current) {
  $string .= "<select name=".$fieldname."_".$part.">\n"; 
  for ($x = $current['start']; $x <= $current['end']; $x++) { 
    if ($key=="mon") $value=$months[$x-1]; else $value=$x;
    if ($x==$today[$part]) $selected="selected"; else $selected="";
    $string .= "<option value=$x $selected>$value</option>\n";
  } 
  $string .= "</select>\n"; 
}
так-то немного покороче будет ,чем у тебя?
 
Войдите или зарегистрируйтесь для ответа.
Сверху