Javascript-форум (https://javascript.ru/forum/)
-   Серверные языки и технологии (https://javascript.ru/forum/server/)
-   -   Оцените код тестов. (https://javascript.ru/forum/server/70773-ocenite-kod-testov.html)

DivMan 01.10.2017 22:36

Оцените код тестов.
 
Сделал тесты, без БД и файлов и ответов нету в браузере.

Попытался сделать структуру массива, приближённую к структуре БД.

Оцените пожалуйста.

Задача:

Дан массив с вопросами и правильными ответами. Пользователь должен выбрать один и вариантов. Когда вопросы заканчиваются - он жмет на кнопку, страница обновляется и вместо вариантов под вопросами появляется сообщение вида: 'ваш ответ: ... верно!' или 'ваш ответ: ... неверно! Правильный ответ: ...'. Правильно отвеченные вопросы должны гореть зеленым цветом, а неправильно - красным.

В массив можно сколько угодно засунуть тестов и в сам код обработки, лезть не придётся.


$questions = [
    1 => [
        'question' => 'Сколько будет 2+2?',
        'variants' => ['2', '4', '6'],
        'answer' => '1'
           
    ],
   
    2 => [
        'question' => 'Сколько будет 5+5?',
        'variants' => ['10', '55', '25'],
        'answer' => '0'
           
    ],
   
    3 => [
        'question' => 'Зимой и летом, одним цветом?',
        'variants' => ['Крокодил', 'трава'],
        'answer' => '0'
    ]
];
 
 
if(empty($_GET['proverka'])) {
    echo '<form action="" method="get">';
    $i = 1;
    foreach($questions as $num) {
        foreach($num as $dano => $v) {
       
                if(gettype($v) == 'array') {
                $j = 0;
                foreach($v as $val) {
                    echo '<label>'.$val.'<input type="radio" name="'.$i.'" value="'.$j.'"></label>&nbsp;&nbsp;&nbsp;';
                    $j++;
                }
            }
            echo '<br>';
           
            if($dano == 'question') {
                echo $v . '<br>';
            }
        }
        $i++;
    }
    echo '<br>';
    echo '<input type="submit" name="proverka" value="Проверить">';
    echo '</form>';
}
 
 
if(!empty($_GET['proverka'])) {
    if(count($_GET)-1 < count($questions)) {
        echo 'Ответьте на все вопросы.';
    }
    else {
        $k = 1;
   
        foreach($questions as $num) {
            foreach($num as $dano => $v) {
                if($dano == 'answer') {
                    if($_GET[$k] == $v) {
                        echo $questions[$k]['question'] .'<br> <span style="color: green; font-weight: bold;">Ваш ответ '.$questions[$k]['variants'][$_GET[$k]].' - правильно</span><br><br>';
                    }
                    else {
                        echo $questions[$k]['question'] .'<br><span style="color: red; font-weight: bold;">Ваш ответ '.$questions[$k]['variants'][$_GET[$k]].' - неправильно, правильный ответ: '.$questions[$k]['variants'][$questions[$k]['answer']].'</span><br><br>';
                           
                    }
                }
            }
            $k++;
        }
    }      
}

рони 01.10.2017 22:57

DivMan,
:victory:

DivMan 01.10.2017 23:16

Чо реально, что ли?

laimas 02.10.2017 00:35

Цитата:

Сообщение от DivMan
foreach($questions as $num) {
foreach($num as $dano => $v) {

if(gettype($v) == 'array') {
$j = 0;
foreach($v as $val) {
echo '<label>'.$val.'<input type="radio" name="'.$i.'" value="'.$j.'"></label>&nbsp;&nbsp;&nbsp;';
$j++;
}
}
echo '<br>';

if($dano == 'question') {
echo $v . '<br>';
}
}
$i++;
}

И зачем такие сложности?

DivMan 02.10.2017 06:08

А как проще?

laimas 02.10.2017 08:28

Цитата:

Сообщение от DivMan
А как проще?

А вы как думаете?
Ассоциация на то и существует, чтобы проще было отождествлять. А у вас что получается - используете ассоциативный массив и при этом пишите такое:

if(gettype($v) == 'array')

Три(!) вложенных цикла для построения формы по заданной ассоциации, это уже перебор.

Кроме этого, без БД - ладно, но разве нельзя использовать как и в БД уникальность? Ведь первичный ключ массива тоже будет уникальным и служить адресом опросов/вопросов/итп. А значит поля формы клиента адресуются по этим уникальным ключам. Вместо этого у вас опять бедлам с кучей условий и двумя вложенными циклами.

Rasy 02.10.2017 12:54

DivMan,
Главное, чтобы смог прочитать свой код в будущем. Из-за большой вложенности ты в нем легко запутаешься.
Работая с массивом, как индексируемым, ассоциативные ключи - лишнее.
Вместо инкрементирования счетчика $i в цикле, в foreach можно определить переменную $key для подобного рода задач.

$questions = [
    [
        'question' => 'Сколько будет 2+2?',
        'variants' => ['2', '4', '6'],
        'answer' => '1'

    ],

    [
        'question' => 'Сколько будет 5+5?',
        'variants' => ['10', '55', '25'],
        'answer' => '0'

    ],

    [
        'question' => 'Зимой и летом, одним цветом?',
        'variants' => ['Крокодил', 'трава'],
        'answer' => '0'
    ]
];

$html = '';

function printInputs($i, $n, $k)
{
    global $html;
    $html .= '<label>'.$i.'<input type="radio" name="'.$k.'" value="'.$n.'"></label>&nbsp;&nbsp;&nbsp;';
}

function printResult($b, $k, $v, $arr)
{
  global $html;
  $msg = '';
  $style = '';

  if ($b) {
    $style = ' style="color: green; font-weight: bold"';
    $msg = 'правильно';
  } else {
    $style = ' style="color: red; font-weight: bold"';
    $msg = 'неправильно, правльный ответ: ' . $arr[$k]['variants'][(int)$arr[$k]['answer']];
  }

  $html .= '<p>'.$arr[$k]['question'].'<br><span '. $style.'>Ваш ответ '.$arr[$k]['variants'][$v].' - '.$msg.' </span></p>';

}

if(empty($_GET['proverka'])) {
    $html .= '<form action="" method="get">';

    foreach($questions as $key => $arr) {
        if (array_key_exists('question', $arr))
          $html .= '<p>' . $arr['question'] . '</p>';

        if (array_key_exists('variants', $arr) && is_array($arr['variants']))
          array_walk($arr['variants'], 'printInputs', $key);

    }
    $html .= '<p><input type="submit" name="proverka" value="Проверить"></p></form>';

} else {

    if (count($_GET)-1 < count($questions)) {
        $html .= 'Ответьте на все вопросы!';
    } else {

      array_pop($_GET);

      foreach ($_GET as $key => $value) {

          if ($value == $questions[$key]['answer']) {
            printResult(true, $key, $value, $questions);
          } else {
            printResult(false, $key, $value, $questions);
          }

      }

    }

}

echo $html;

Rasy 02.10.2017 12:57

DivMan,
Да, и читай разные методы в доках и в каких случаях их лучше использовать.

laimas 02.10.2017 13:07

Цитата:

Сообщение от Rasy
ассоциативные ключи - лишнее

Это не так. И ваш тоже не блещет.

Rasy 02.10.2017 13:18

Лентяй, ok

laimas 02.10.2017 13:28

Цитата:

Сообщение от Rasy
Ты лентяй, а мне мнение лентяя не особо помогает. Лучше минусуй меня, как Rise и будь счастлив.

Не надо впадать в детство - это совет. Я уже взрослый человек, такими категориями не мыслю.

Цитата:

Сообщение от Rasy
Ты из категории зубрил

Не важно к какой категории я отношусь, но вы пишите мягко выражаясь ахинею, а не код. Ну неужто так трудно даже имея вместо sql-таблицы массив поступать так же по логике как и в случае с БД?

Я ничего и не намерен писать, я знаю, что автор поста и с БД тренировался, и знает по крайней мере принципы, а значит если подумает маленько, то способен будет написать более логичную структуру массива и соответственно код с ней работающий.

А вам же подумать над своим:

(int)$arr[$k]['answer'] - это да, необходимо, но посмотрите как при этом организованы ваши данные. А они организованы так, что intval к ним как издевательство. Не буду пояснять почему, сами догадайтесь, вы же в отличие от меня из категории умельцев.

Rasy 02.10.2017 13:38

Лентяй, ok

DivMan 03.10.2017 15:39

Я усложнил задание.

Теперь на один вопрос может быть несколько правильных ответов. Пользователь должен отметить один или несколько чекбоксов.

$questions = [
	1 => [
		'question' => 'Сколько будет 2+2?',
		'variants' => ['2', '4', '6', '4.0'],
		'answers' => ['1', '3']
			 
	],
	
	
	2 => [
		'question' => 'Сколько будет 6+6?',
		'variants' => ['66', '6', '12'],
		'answers' => ['2']
			 
	],
	
	
	3 => [
		'question' => 'Найдите несуществующие месяцы',
		'variants' => ['Февраль', 'Июндр', 'Сентябрь', 'Авгурст', 'Денабрь'],
		'answers' => ['1', '3', '4']
			 
	],
	
	
];


if(empty($_GET['proverka'])) {
	echo '<form action="" method="get">';
	$i = 1;
	foreach($questions as $num) {
		foreach($num as $dano => $v) {
		
				if(gettype($v) == 'array' && $dano == 'variants') {
				$j = 0;
				foreach($v as $val) {
					echo '<label>'.$val.'<input type="checkbox" name="'.$i.'[]" value="'.$j.'"></label>&nbsp;&nbsp;&nbsp;';
					$j++;
				}
			}
			echo '<br>';
			
			if($dano == 'question') {
				echo $v . '<br>';
			}
		}
		$i++;
	}
	echo '<br>';
	echo '<input type="submit" name="proverka" value="Проверить">';
	echo '</form>';
}


if(!empty($_GET['proverka'])) {

	if(count($_GET)-1 < count($questions)) {
		die ('Ответьте на все вопросы.');
	}
	
	
	$k = 1;
	foreach($questions as $num) {
		foreach($num as $dano => $v) {
			
			if($dano == 'answers') {
				$rightAnswers = array_diff($_GET[$k], $questions[$k]['answers']);
				$countUserAnswer = $_GET[$k];
				$countAnswer = $questions[$k]['answers'];
		
				echo '<br>';
				
				if(count($rightAnswers) == 0 && ($countUserAnswer == $countAnswer)) {
				
					$keyAnswer = [];
					
					foreach($_GET[$k] as $userAnswer) {
						$keyAnswer[] = $questions[$k]['variants'][$userAnswer];
					}
					
					echo $questions[$k]['question'] . '<br>';
					
					echo 'Ваш ответ: ' ;
					echo implode(', ', $keyAnswer);
					echo ' и это <span style="color: green; font-weight: bold;">правильный ответ</span>.';
					
					echo '<br><br>';
				}
				
				
				else {
					$keyAnswer = [];
					$keyTrueAnswer = [];
				
					echo $questions[$k]['question'] . '<br>';
					
					echo 'Ваш ответ: ' ;
					
					foreach($_GET[$k] as $userAnswer) {
						$keyAnswer[] = $questions[$k]['variants'][$userAnswer];
					}
					
					echo implode(', ', $keyAnswer);
					
					echo ' - <span style="color: red; font-weight: bold;">это неправильно</span>.<br>';
					echo 'Правильный ответ(ы): ' ;
					
					foreach($questions[$k]['answers'] as $ans) {
						$keyTrueAnswer[] = $questions[$k]['variants'][$ans];
					}
					
					echo implode(', ', $keyTrueAnswer);
					
					echo '<br><br>';
				}
				
				
				
			}
			
		}
		$k++;
	}
	
}

laimas 03.10.2017 19:44

DivMan,
вы же как-то тренировались с базами и знаете, что в разумной БД уникального идентификатора равного 0 не будет. Если опираясь на это, делать также и с применением массива, то упрощается проверка мусора/опасного.

Первичный индекс вашего массива данных этому соответствует. Правда в РНР можно поступать проще - задать только первому элементу индекс, остальные получат его по возрастанию автоматически, то есть:

$questions = [
    1 => [
        'question' => 'Сколько будет 2+2?',
        'variants' => ['2', '4', '6', '4.0'],
        'answers' => ['1', '3']
              
    ],
    [
        'question' => 'Сколько будет 6+6?',
        'variants' => ['66', '6', '12'],
        'answers' => ['2']
              
    ],
.....


А вот дальше все неверно. У вас есть записи в базе, которые имеют уникальные идентификаторы и названия. По этим данным вы строите список, что в качестве value будут содержать опции списка? Можно быть уверенным, что идентификаторы, которые не имеют нулевого значения.
А почем в массиве данных не так - [1=>'2', '4', '6', '4.0']?

Именование полей цифрами, это плохо, могут быть и косяки, вообще имена должны начинаться с буквы. Для построения формы по вашим данным требуется максимум два массива, причем вы определяет данные, и догадываться проверкой что они пришли это плохо, и виноват у вас в этом как раз лишний цикл. А должно быть

Первый цикл - обход массива $questions как ключ=>значение, вложенный цикл как значения берет из значения ключ 'question' и выводит метку поля, берет ключ 'variants' и выводит кнопки формы именуя их как, например, variants[первичный ключ массива $questions] а value индексы variants, которые начинаются с 1. Зачем тут третий цикл?

И проверка буде без всяких if(count($_GET)-1 < count($questions)), что вообще ни к чему, условием будет if(count($_GET) == count($questions)), а для этого не давайте имя кнопки отправки формы, она у вас одна в форме и серверу совсем не нужна. Достаточно проверить наличие ключа variants в массиве $_GET. А проверить правильно или нет, два цикла совсем не нужны, достаточно одного.

DivMan 03.10.2017 23:48

Спасибо за рекомендации

laimas 04.10.2017 03:42

Рекомендации в руководстве. )
А вы же задав ключи даже забудьте о ереси $i = 1; name="'.$i.'[]", $i++; $k = 1; $_GET[$k], $k++;

Если вы решите еще более расширить это приложение, то возможно появится и управление. Возможно, что одни задачки будут удалены, а другие добавлены. А уникальность означает не только "не повторяющийся", но еще и "используемый единожды" - если будет удалена задачки под индексом 2, то вновь добавляемая ни в коем случае не должна получать этого индекса, только следующий за максимальным в наборе. Какие могут быть $i++?

И проверять нужно истинность входных данных, и в данном случае это можно делать просто:

if($get = array_intersect_key($_GET, $questions) AND count($get) == count($questions)) {
    //данные истинны и выбрано все
    //и один единственный цикл для разбора входных данных
    foreach($get as $key=>$val) {    
        if((int)$val == $questions[$key]['answers']) {
            //данные истинны и ответ правильный
        }
    }
}

DivMan 04.10.2017 09:43

Если делать по примеру БД, но наверно придётся создавать для каждого ответа, отдельный ключ?

laimas 04.10.2017 09:57

Цитата:

Сообщение от DivMan
придётся создавать для каждого ответа, отдельный ключ?

В каком смысле? Индекс массива, это и есть ключ, чем он хуже первичного ключа sql-таблицы? Вот только индекс должен начинаться с 1, тогда и проверка на истинность упрощается.

Но ваш код имеет одну проблему, его структура неверна. А почему, найдите ответ сами.

DivMan 04.10.2017 10:59

вот так должно быть?

$questions = [
    1 => [
        'question' => 'Сколько будет 2+2?',
        'variant1' => '2', 
        'variant2' => '4',
        'variant3' => '6',
        'answer1' => '1',
        'answer2' => '2',
              
    ],

laimas 04.10.2017 11:11

Цитата:

Сообщение от DivMan
вот так должно быть?

Нет, и уже говорилось - именование ключей одного набора как name1, name2, ... это геморрой. Должно быть 'variants' => [1 => '2', '4', '6', '4.0'] и работа с индексами этого массива как с уникальными значениями одного набора.

Я говорю о структуре кода в целом, а не частных его составляющих, в которых много лишнего у вас и необдуманного. А общая структура кода не отвечает решаемой задаче, подумайте почему?

DivMan 04.10.2017 11:59

Но работает же всё, как надо?

laimas 04.10.2017 12:31

Цитата:

Сообщение от DivMan
Но работает же всё, как надо?

Так ли? Подумайте над тем, что должен возвращать сервер если пользователь ответил не на все вопросы.

DivMan 04.10.2017 15:59

Будет сообщение Ответьте на все вопросы

laimas 04.10.2017 16:20

Цитата:

Сообщение от DivMan
Будет сообщение Ответьте на все вопросы

И все? Ну это почти как "послать на..." :)

Структура кода должна быть таковой:

Первый блок, это проверка отправления формы клиентом. Если форма пришла, обрабатываем и если есть ошибки генерируем их, в противном случае генерируем "Вы выиграли кепку", выводим кнопку "Хотите еще?" и т.п. и завершаем работу скрипта.

А форма выводится следующим блоком кода и по двум условиям - если не было приема формы, то есть первичный запрос страницы и если был прием формы, и есть ошибки. При этом наряду с формой выводятся и ошибки допущенные, а форма должна отражать сделанный пользователем выбор, то есть для радио кнопок не просто <input параметры>, а если кнопка выбиралась пользователем, то checked.

DivMan 04.10.2017 20:22

то есть, если пользователь ответил не на все вопросы, то вывести результат на те вопросы, на которые он ответил и сообщение, на которые он ещё должен ответить?

laimas 04.10.2017 22:29

Цитата:

Сообщение от DivMan
если пользователь ответил не на все вопросы, то вывести результат на те вопросы, на которые он ответил

Зачем? Если сервер ожидает ответ на все вопросы, а приходит не на все, это не финал, а ошибка. Значит вывод ошибки и формы, в которой сделанный пользователем выбор никуда не пропал.

Приходилось заполнять бумажки в разных инстанциях? А если:

- Вот тут не заполнено.

бланк над которым вы корпели при это разрывают, а за новым надо возвращаться в очередь. Это нормально?

DivMan 04.10.2017 22:42

Надо сделать, чтобы чекбокс был активным, если есть такой элемент в массиве гет?

laimas 04.10.2017 23:42

Да.


Часовой пояс GMT +3, время: 21:34.