2
\$\begingroup\$

I have been learning PHP and wanted to see if I could make a very simple slot machine game. Everything works, but I'm sure this is not the best way to do this. Please let me know how you would do this and what I can do to improve.

<?php

$num1 = rand(1, 5);
$num2 = rand(1, 5);
$num3 = rand(1, 5);

$result = $num1.' | '.$num2.' | '.$num3;

// Read points from file
$filename = 'points.txt';
$handle = fopen($filename, 'r');
$current = fread($handle, filesize($filename));
fclose($handle);

if ($num1 == $num2 && $num2 == $num3) {
$status = '<big>You are a winner!</big>';

$add_points = $current + 10;

// Add points to file
$handle = fopen($filename, 'w');
$current_points = fwrite($handle, $add_points);
fclose($handle);

} else {
$status = 'please try again!';
}

?>
<html>
<head>
<title>Slot Machine Game!</title>
</head>
<body>

<center><big><?php echo $result; ?></big></center>
<br />
<br />
<center><big><?php echo $status; ?></big></center>
<br />
<br />
<center><big><?php echo 'You have <strong>'.$current.'</strong> points!'; ?></big></center>

</body>
</html>
\$\endgroup\$
  • \$\begingroup\$ Bah: you deleted your question on Stack Overflow whilst I was writing out my answer, so I had to create an account here so as not to waste it! Once a question is asked, it is good practice not to delete it yourself - let a mod migrate it if necessary. \$\endgroup\$ – halfer Mar 10 '12 at 19:13
  • \$\begingroup\$ To add to what halfer said, if you put your question on the wrong site, please wait for it to be migrated or closed by the moderator. They can usually transfer the question to the correct site for you saving everybody effort. \$\endgroup\$ – Winston Ewert Mar 10 '12 at 19:18
  • \$\begingroup\$ Oh okay, sorry about that. \$\endgroup\$ – Nick Meagher Mar 10 '12 at 20:19
  • \$\begingroup\$ Wouldn't hurt to put an srand() in before you start using rand() \$\endgroup\$ – deed02392 Mar 22 '12 at 10:48
3
\$\begingroup\$

Looks good. Few initial things:

  • Indent inside control structures, so everything inside your if() statement would be prefixed by one tab. (You can use a fixed number of spaces, but that can cause problems if you share your code with people who use a different tab-spacing system).
  • It is a really good idea to split out your code into logic (or "controller") and template (or "view"). Your html would then go in a different file and be loaded by your PHP file - a good first step with this sort of thing is Smarty. From there you can progress to a proper PHP framework such as Symfony or Zend (but be aware the learning curve is steep).
  • Tags such as <center> and <big> tend to be discouraged - use CSS instead.
  • Keep your PHP statements in your view as small as possible. For example I'd rewrite your score thus:
<div class="points">
    You have <strong><?php echo $current ?></strong> points!
</center>

You'll need to define a CSS class called 'points', of course.

\$\endgroup\$
  • \$\begingroup\$ Yeah, if I was writing this seriously I would have taken all those steps but I just threw it together for the sake of example so it was easier to understand on your guys end, but thanks for all the feedback! \$\endgroup\$ – Nick Meagher Mar 10 '12 at 20:23
  • 2
    \$\begingroup\$ Oh dear - after all my effort, and you didn't really need the feedback? </depressed> \$\endgroup\$ – halfer Mar 10 '12 at 20:44
  • \$\begingroup\$ Well im new to PHP but I have been doing HTML and CSS for a couple years now so I knew some of that stuff but I was just trying to confirm that this was written okay or if there was another way I could do it like save it in a cookie instead of a flat file, this is the only way i know. I have looked at MVC frameworks but don't completely understand them yet, the only things I have used are Wordpress and Drupal \$\endgroup\$ – Nick Meagher Mar 10 '12 at 20:52
3
\$\begingroup\$
<?php

$num1 = rand(1, 5);
$num2 = rand(1, 5);
$num3 = rand(1, 5);

$result = $num1.' | '.$num2.' | '.$num3;

// Read points from file
$filename = 'points.txt';
$handle = fopen($filename, 'r');
$current = fread($handle, filesize($filename));

current is kinda vauge as a name. Is there something better you can call it.

fclose($handle);

Php has a function file_get_contents which will do those last four lines for you.

if ($num1 == $num2 && $num2 == $num3) {
$status = '<big>You are a winner!</big>';

Please indent inside of braces. It makes code way easier to read!

$add_points = $current + 10;

Why not simple add to $current?

// Add points to file
$handle = fopen($filename, 'w');
$current_points = fwrite($handle, $add_points);
fclose($handle);

use file_put_contents

} else {
$status = 'please try again!';
}

?>
<html>
<head>
<title>Slot Machine Game!</title>
</head>
<body>

<center><big><?php echo $result; ?></big></center>
<br />
<br />
<center><big><?php echo $status; ?></big></center>
<br />
<br />
<center><big><?php echo 'You have <strong>'.$current.'</strong> points!'; ?></big></center>

Why not:

<center><big>You have <strong><?php echo $current; ?></strong> points!</big></center?

It seems easier to follow.

</body>
</html>
\$\endgroup\$
  • \$\begingroup\$ Yeah my indenting is totally wrong on here because I was a little confused about how to insert the code on here, it looks better in my code editor. Yeah I went back and looked for what I could do better and I made the those changes, I had no idea what file_get_contents did but now I do, thanks! \$\endgroup\$ – Nick Meagher Mar 10 '12 at 20:22
  • \$\begingroup\$ @NickMeagher, the indenting issue was because you used tab characters. Your editor should have an option to use spaces which will make copy/pasting into places like this easier. \$\endgroup\$ – Winston Ewert Mar 10 '12 at 23:32
-1
\$\begingroup\$

Maybe you know but this game can only be played by one player at a time. Also there is no way for that player to restart there game.

As you are using this to learn exercise I would suggest your next step would be to use PHP sessions to store the players score. Then give the user the ability to start a new game.

It would also be nice to add a "Play Again" button and play again only if that button was click. If the user just refreshed try to have it not play again.

If you decide to go that way then you could have a new player welcome message for users who have not started to play yet.

Good luck.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.