Problems Displaying Multiple Matching Rows

MySQL database
#1

I am writing a wrestling promoter sim type game and am stuck on how to do something fairly common.

The page in question displays information pulled from multiple MySQL tables. As it stands everything works on this page except I’m not sure how to create a list of all rows in the “moves” table that match the wrestler. Here’s the code:

$league = $_GET['league'];
$wrestler = $_GET['wrestler'];

require_once("dbc.php");
$sql = "select roster_singles.*, roster_tag_teams.team_name, moves.move_name 
	from roster_singles
	inner join roster_tag_teams 
		on (roster_singles.tag_team = roster_tag_teams.team_id and roster_singles.wrestler_id = '".$wrestler."')
	inner join moves
		on (roster_singles.wrestler_id = moves.wrestler_id and moves.wrestler_id = '".$wrestler."')";
$result = $mysqli->query($sql);

$row = $result -> fetch_assoc();
printf("<p><center><h2>%s</h2></center><br><br>", $row['ring_name']);
printf("<p><h3>Vital Info</h3><br><br>");
printf("<p><b>Name:</b> %s<br>", $row['ring_name']);
printf("<b>Location:</b> %s<br>", $row['location']);
printf("<b>Height:</b> %s<br>", $row['height']);
printf("<b>Weight:</b> %s<br>", $row['weight']);
printf("<b>Entrance Music:</b> %s<br>", $row['music']);
switch ($row['status']) {
	case "a": 
		$status = "Active";
		break;
	case "i":
		$status = "Injured";
		break;
	case "x":
		$status = "Inactive";
		break;
	default:
		$status = "Unknown";
		break;
	}
printf("<b>Status:</b> %s<br>", $status);
printf("<b>Tag-Team:</b> %s<br>", $row['team_name']);

printf("<p><h3>Moves</h3><br><br>");
printf("<p><ul>");
while($row = $result->fetch_assoc()) {
	printf("<li>%s (%s)", $row['move_name'], $row['description']);
	}
printf("</ul>");
}

The moves table for each wrestler will have a minimum of one move per wrestler, but can have an unlimited number. However, the above code only shows the first matching row. I’m not sure how to get it to list one row per move. Any guidance on this problem, or my coding in general is appreciated. Thanks!

#2

I recommend that you move the database specific code so that it is above the start of the html document and fetch all the data from the query into an appropriately named php array variable. This will -

  1. Make it easier to test your application (you can print_r()/var_dump() the data so that you can make sure it is what you expect.)
  2. Simplify the code in the html document.
  3. Allow you to display a message if the query didn’t match any data.
  4. Allow you to access the zeroth row of data to display the one-time/unique information about a result set.
  5. Allow you to loop over all the rows of data, without loosing any of the rows. Your current code is fetching the first row of data to display the one-time/unique information, but is not displaying the move information from that row, then looping to display the move information from the remainder of the rows in the result set.

Here’s a laundry list of things that will make the code more secure and simpler -

  1. You should trim, then validate all input data before using it.
  2. If the wrestler input is ‘required’, you need to setup message for the user, rather than to continue to use the input to try to find matching data, if it is empty.
  3. Don’t put external, unknown, dynamic values directly into an sql query statement, where any sql special characters in a value can break the sql query syntax, which is how sql injection is accomplished. Use a prepared query instead. This would also be a good time to switch to the much simpler and more modern PDO database extension.
  4. Use ‘require’ for things you code must have for it to work, and require isn’t a function, the () around the filename are unnecessary clutter.
  5. Use table alias names in your query, such as rs (for roster_singles), rt, and m. This will simplify all the typing.
  6. List out the columns you are selecting. This help make your query self-documenting and helps prevent mistakes.
  7. The printf() statements aren’t adding anything useful and are just more typing for you.
  8. Any dynamic value that you output on a web page should have htmlentities() applied to it to help prevent cross site scripting.
  9. Rather than to write out conditional logic for each possible value (the switch/case statement), use a database table when mapping input (which should be an id) to output values. You can then just JOIN the status table in the query.
#3

Thanks. I am new to coding outside of a game engine, which took care of the security aspects, and I have yet to find a suitable replacement. I will switch to pdo and clean up my code. I’m sure I’ll revisit this topic once I have this cleaned up.

Sponsor our Newsletter | Privacy Policy | Terms of Service