Ik ben bezig met een delete functie waarbij ik uit 3 tabellen gegevens wil wissen. Eerst heb ik het ID van de bepaalde speler opgezocht in mijn database en daarna wil ik alle gegevens met die ID verwijderen. Dit is mijn code maar het werkt niet, kan iemand mij helpen?
<?php
require('connection.php');
$player = $_POST['player_name'];
$query = "SELECT player_id FROM player WHERE player_name = '$player';";
$resultaat = mysqli_query($connectie, $query);
$rij = mysqli_fetch_array($resultaat);
$queryverwijderplayer = "DELETE FROM player WHERE player_id = $rij['player_id'];";
echo $queryverwijderplayer;
?>
Aan de bovenstaande code rammelt nog van alles:
- input wordt niet gefilterd/gevalideerd, ook zou het beter zijn om direct met een id te werken (zie toelichting verderop)
- output zou ge-escaped moeten worden zoals @Ariën aanhaalt, op dit moment is SQL-injectie mogelijk; misschien is het met wat kunst- en vliegwerk zelfs mogelijk om alle of willekeurige spelers op deze manier te verwijderen
- dit zou voor de goede orde misschien in een transactie moeten staan
- je controleert niet eens of de eerste query een resultaat oplevert maar je gaat er blindelings vanuit dat die query een resultaat heeft
Het enige wat effectief is veranderd is de verwijdering van punt-komma's in de queries en het toevoegen van quotes om het player id?
Maar los daarvan, de hele opzet is logisch gezien... niet erg logisch. Ik bedoel, als je dan toch een selectie doet op spelersnaam, en vervolgens die speler dan op grond van het bijbehorende id verwijdert, waarom verwijder je dan niet meteen een speler op basis van spelersnaam? Of dus beter, door alleen te werken met speler ids, zodat er nooit een naam aan te pas komt, wat de kans op het verwijderen van een verkeerde speler verder wegneemt.
En dan het verwijderen zelf, misschien is dat ook geen goed idee. Voor wanneer er toch per ongeluk de verkeerde speler wordt verwijderd. Zou het niet beter zijn om een status "deleted" bij te houden, zodat "verwijderde" spelers eventueel ook weer teruggehaald kunnen worden?
Het enige wat effectief is veranderd is de verwijdering van punt-komma's in de queries en het toevoegen van quotes om het player id?
Volgens mij vroeg Max toch echt waarom het niet werkte.
Om de code te laten werken was dat nodig. Dus, what is your point?
Ik geef er nog bij aan dat hij eigenlijk eerst een controle moet doen om te kijken of er überhaupt is gepost, en ook nog dat het lek is. Ja, het kan allemaal beter. Is het goed om advies te geven? Zeker.
Maar je ziet toch ook wel dat Max daar nog niet zover in is....
Volgens mij vroeg Max toch echt waarom het niet werkte.
Om de code te laten werken was dat nodig. Dus, what is your point?
Nou, je gaf alleen een nieuw codefragment zonder enige toelichting. Wat kan de topicstarter daar van leren? Dit is toch geen "zoek de 10 verschillen" website?
Bart V B op 06/11/2018 19:15:34
Ik geef er nog bij aan dat hij eigenlijk eerst een controle moet doen om te kijken of er überhaupt is gepost, en ook nog dat het lek is. Ja, het kan allemaal beter. Is het goed om advies te geven? Zeker.
Maar je ziet toch ook wel dat Max daar nog niet zover in is....
Reden te meer om te onderbouwen wat je doet, en nog belangrijker, waarom. En ook om op een gegeven moment een waardeoordeel te geven over de code waar je mee bezig bent. In dit geval lijkt het mij ofwel verstandiger om opnieuw te beginnen, of dit maar te laten rusten. Als dit slechts een fragment is van een veel grotere applicatie, grote kans dat daar ook enorme gaten in zitten, wat heeft het dan voor zin om dit stukje op te lappen?
Dankjewel voor de reacties, ik ben inderdaad nog een beginner in PHP. De my_sqli error zit er al in bij connection.php dus die heb ik dan niet nodig neem ik aan. Ik heb nu gezorgd dat de value van de vorige pagina het ID van de speler is in plaats van de naam, daarmee heb ik dit gekregen
<?php
require('connection.php');
$player = mysqli_real_escape_string( $connectie, $_POST['player_name']);
$queryverwijderplayer = "DELETE FROM player WHERE player_id = $player;";
$queryverwijderclub = "DELETE FROM club WHERE player_id = $player;";
$queryverwijdertime = "DELETE FROM time WHERE player_id = $player;";
$resultaatplayer = mysqli_query($connectie, $queryverwijderplayer);
$resultaatclub = mysqli_query($connectie, $queryverwijderclub);
$resultaattime = mysqli_query($connectie, $queryverwijdertime);
header('Location: beveiligdepagina.php');
exit();
?>
Ja, maar gebruik van die() is niet netjes.
Wij sterven toch ook niet als we wat fout doen? ;-)
Ikzelf gebruik de Object-georienteerde versie van MySQLi (met pijltjes, waaronder: $db->query(.....) ). In een MySQL extended class overschrijf ik de query() functie en voeg daar standaard foutafhandeling toe (als Exception).
Vanaf dan heb ik bij elke query een foutafhandeling zonder dat ik ze hoef te verpakken met if-statements.