[web] another php problem, sry guys (solved)

Started by
12 comments, last by bigfooot 18 years, 5 months ago
Ah ha!

WHERE userid='$userstatsall3[userid]'"; <-- You can't do that

instead, try

$userid = $userstatsall3['userid'];

$getfans="UPDATE userstats SET money=money-'$total', fans=fans+'$fanswanted', WHERE userid='$userid'";


And I don't know any good websites on making good php code, but I do have a good book or two. PHP and Mysql web development by Luke Wellington and Laura Thomson is quite good.

But, as a general practice... anything you're going to write more than once needs to be a function. For example... all of this:

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<title>buy fans</title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
</head>


could be written as a single function. It would look like this:

function make_html_header($Title)
{

?>

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<title><?php echo $Title; ?></title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
</head>


<?php

}

That way, all you'd need to do is call make_html_header('PageTitle') at the top of each page, and it would save you several lines of code and a ton of redundancies.

Also, you need to find better names for your variables ($userstatsall3 has no implied meaning what-so-ever). If it were just $UserStats, you'd be ok. But 4 months from now when you're going through your code.... you're not going to remember what the 3 indicates.

In addition, you need to capitalize the first letter of each word. $UserStatsAll_3 would at least be easier to read than $userstatsall3.

Last but not least, avoid nested if statements. Sometimes, its unavoidable, but (IMO) its good practice not to use them if possible. You'd be 10 times better off storing both the username and password in a session variable, creating a function called 'loggedin' that tests the username/password combo to see if its valid, then throwing an error and exiting the script if loggedin returned false.

There are about dozen or so other things I'd do differently.... but as is, someones already going to pick a fight by tomorrow mourning telling me that nested ifs are the greatest invention and that I'm out of my mind. Mostly its a matter of preference, but I'd start now with making your code as clean and as efficent (for both you and others to read) as possible before trying to expand into a 12000 line program.
Advertisement
thanks for your reply, but the script still does not work. Here is the newest version of the code:

<?phpinclude 'db.php';session_start();ini_set ('display_errors', 1);error_reporting (E_ALL & ~E_NOTICE);echo $_SESSION['userid'];if (isset($_SESSION['userid'])) 	{    $userid=$_SESSION['userid'];    $userstatsall="SELECT * from userstats where userid='$userid'";    $userstatsall2=mysql_query($userstatsall) or die("Could not get user stats");    $userstatsall3=mysql_fetch_array($userstatsall2);	$userid = $userstatsall3['userid'];    if(isset($_POST['submit']))    {		$fanswanted = (int) $_POST['fansamount'];		$fanswanted=strip_tags($fanswanted);		$total=$fanswanted*100;		if($fanswanted<0)		{		print "You cannot buy negative fans.
.";
}
else if($userstatsall3['money']<$total)
{
print "You do not have enough money to buy that many fans.";
}
else if($userstatsall3['money']>=$total)
{
$getfans="UPDATE userstats SET money=money-'$total', fans=fans+'$fanswanted', WHERE userid='$userid'";
mysql_query($getfans) or die("Could not buy fans");
print "You bought $fanswanted fans. .";
}
else
{
print "there was an error";
}
}
}
else
{
print "Sorry, not logged in.";
}

print "test"

?>


And here is the database dump, in case it is usefull:

# phpMyAdmin SQL Dump# version 2.5.3# http://www.phpmyadmin.net## Host: localhost# Generation Time: Oct 22, 2005 at 03:50 PM# Server version: 4.0.15# PHP Version: 4.3.3# # Database : `databasev001`# # --------------------------------------------------------## Table structure for table `users`#CREATE TABLE `users` (  `userid` int(25) NOT NULL auto_increment,  `first_name` varchar(25) NOT NULL default '',  `last_name` varchar(25) NOT NULL default '',  `email_address` varchar(25) NOT NULL default '',  `username` varchar(25) NOT NULL default '',  `PASSWORD` varchar(255) NOT NULL default '',  `info` text NOT NULL,  `user_level` enum('0','1','2','3') NOT NULL default '0',  `signup_date` datetime NOT NULL default '0000-00-00 00:00:00',  `last_login` datetime NOT NULL default '0000-00-00 00:00:00',  `activated` enum('0','1') NOT NULL default '0',  PRIMARY KEY  (`userid`)) TYPE=MyISAM COMMENT='Membership Information' AUTO_INCREMENT=2 ;## Dumping data for table `users`#INSERT INTO `users` VALUES (1, 'my name', 'my lastname', 'My email', 'Username', 'MYPASSWORD', 'MYINFO', '0', '2005-10-20 09:01:28', '2005-10-22 15:44:39', '1');# --------------------------------------------------------## Table structure for table `userstats`#CREATE TABLE `userstats` (  `userid` int(25) NOT NULL auto_increment,  `money` int(25) NOT NULL default '1000',  `fans` int(25) NOT NULL default '15',  PRIMARY KEY  (`userid`)) TYPE=MyISAM AUTO_INCREMENT=2 ;## Dumping data for table `userstats`#INSERT INTO `userstats` VALUES (1, 1000, 10);
Is it still saying "Could not buy fans" or does it give you another error?

Also, why do you have:

$fanswanted=strip_tags($fanswanted);

Isn't $fanswanted suppose to be an integer? If so, you need to have a function that checks to see if a value is an integer or not. It should return either true or false.


Also, the statment:

$getfans="UPDATE userstats SET money=money-'$total', fans=fans+'$fanswanted', WHERE userid='$userid'";

should not have a comma after '$fanswanted'.



YES YES YES YES

Thank you Cygnus_X so much, it works now. Its because of the comma after the $fanswanted.

Ill neaten up the code now and stuff.

Thank you also to all the other guys who helped.

alex

This topic is closed to new replies.

Advertisement