Witam,
dostałem takie zadanie od nauczyciela. ja oczywiście bym tu nic nie zmienił ale mam nadzieje że wy pomożecie mi co można tutaj zmienić:

Treść zadania:
Poniższy skrypt interpretuje dane dla wysłanego formularza
Napisz co byś zmienił w tym kodzie i dlaczego.

Zmiany proszę nanosić na kod + opisać dokładnie co jest zrobione i dlaczego za pomocą komentarzy.

Kod:

[PHP] pobierz, plaintext 
<?php
 
$operation = $_POST['option'];
$id_record_used_later_for_changes_in_table = $_GET['id'];
$type = $_GET['user_type']; 
 
$adminIdList = array(1, 2, 3);
$key = 'form_test'; //!! Debug only
 
echo '<table><tr><th>UserID</th><th>User Name</th></tr>';
 
switch ($operation) {
	case 1:
 
        DbEngine::Instance()->Query("SELECT id, username FROM table_users WHERE type = $type");
        $arrResult = DbEngine::Instance()
            ->FetchAll()
            ->Execute();
 
        for ($i = 1; $i < count($arrResult); $i++) {
            $row = &$arrResult[$i];
            if (in_array($row['username'], $adminIdList)) {
                //Skip admins
            }else{
                echo '<tr><td>' . $row['id'] . '</td><td>' . $row['username'] . '</td></tr>';
            }
        }
 
	case "2":
        $arrToUpdate = array();
        foreach ($_POST as $val) {
            if (strpos($key, 'form_') == 0) { 
                $fieldName = substr($key, 4);
                $arrToUpdate[$fieldName] = $val;
            }
        }
 
        $result = DbEngine::Instance()
            ->Table('tablu_users')
            ->Update()
            ->Where(array('id' => $id_record_used_later_for_changes_in_table))
            ->Set($arrToUpdate)
            ->Execute();
 
        echo 'Update executed successfully.';
 
    case "3":
        $arrToInsert = array();
        foreach ($_POST as $val) {
            if (strpos($key, 'form_') == 0) { 
                $fieldName = substr($key, 4);
                $arrToInsert[$fieldName] = $val;
            }
        }
 
        echo 'Insert executed successfully.';
 
}
 
 
echo '</div>';
[PHP] pobierz, plaintext 

div na koncu nie zamyka nic
id_record_used_later_for_changes_in_table, dluzszej nazwy zmiennej nie moglo byc?

w ogole

[PHP] pobierz, plaintext 
$operation = $_POST['option'];
$id_record_used_later_for_changes_in_table = $_GET['id'];
$type = $_GET['user_type']; 
[PHP] pobierz, plaintext 

nie ma sensu zupelnie, co to zmieni ze zrobisz nowa zmienna i przypiszesz do niej wartosc

sposob na pomijanie adminow tez dobry:D sql do zmiany select user where user_type != admin, dzieki temu nie bedziesz musial dawac if'a w petli

to ma byc podchwytliwe ? : P

"case: 3" - gdzie ten insert został wykonany

Tak, naprawdę jest to test rekrutacyjny, do pracy jak programistas php, jeśli zdamy dostaniemy się na rozmowę rekrutacyjna.

Ja odpisałem że nie zmienił bym nic.

Ale też zauwżyłem że brak jest inserta do bazy.

No nic muszę szukać dalej widać jeszcze nie znalazłem odpowiedniego pracodawcy... albo jestem za cienki...

naprawde nic bys nie zmienil ? to ciekawe. Nie zastanawiajac sie dluzej bym wywalil polowe

[PHP] pobierz, plaintext 
 for ($i = 1; $i < count($arrResult); $i++) {
[PHP] pobierz, plaintext 

No nawet tego byś nie zmienił?

juz 4 linijka jest ciekawa. $id_record_used_later_for_changes_in_table <- seriously ? piszac takie zmienne na pewno projekt bedzie wygladal dobrze ;p

Tak nie dokońca zamiast tego switach można by dodać instację klasy, która zajmuje się dodawaniem, aktualizacją, wyświetlaniem, usuwaniem danych.

Coś w deseń tego pseudo kodu:

[PHP] pobierz, plaintext 
$DB = new DB("","","","");
$DB->add();
$DB->update();
$DB->delete();
$DB->insert();
[PHP] pobierz, plaintext 

Ale np:
na długość: $id_record_used_later_for_changes_in_table nie zwróciłem uwagi, a wiem że jest to nie zalecane pisanie długich zmiennych.
A tego: for ($i = 1; $i < count($arrResult); $i++) { nie zauważyłem. Nie wiem jak to działa i na co to zamienić.

Tak na marginesie, to jeszcze breaków brakuje, ale to tam "drobny" szczegół

Break zauważyłem, ale jak mówisz drobny szczegół...

Widać brakuje mi jeszcze doświadczenia,

[PHP] pobierz, plaintext 
 
$arrToInsert = array();
foreach ($_POST as $val) {
if (strpos($key, 'form_') == 0) {
$fieldName = substr($key, 4);
$arrToInsert[$fieldName] = $val;
}
}
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
 
$arrToUpdate = array();
foreach ($_POST as $val) {
if (strpos($key, 'form_') == 0) {
$fieldName = substr($key, 4);
$arrToUpdate[$fieldName] = $val;
}
}
[PHP] pobierz, plaintext 

calkiem podobne te kody ;>

Dodatkowo strpos zwraca FALSE jak nie znajdzie i 0 jak znajdzie na pozycji zerowej. Podpowiem (FALSE == 0), ale FALSE !== 0

Tak koledze @ohm nie chodziło raczej dosłownie, że jest to drobny szczegół. To "drobny" szczegół. Ten szczegół rozwala całą logikę tego kodu