Feedback on my first PHP script (fibonacci sequence)

  • (2 Pages)
  • +
  • 1
  • 2

16 Replies - 14019 Views - Last Post: 17 August 2011 - 05:57 PM Rate Topic: -----

#1 JoshD  Icon User is offline

  • D.I.C Head

Reputation: 9
  • View blog
  • Posts: 111
  • Joined: 22-March 10

Feedback on my first PHP script (fibonacci sequence)

Posted 16 August 2011 - 06:14 AM

Hello, I'm just looking for a little feedback on my first PHP script please. ( I don't know why it tells us not to say please in our topic title, anyywaayyy...).

So basically I've read through a book but this is the first bit of PHP that I have written.
I know it works, so what I'd like to know is:
  • Is there any bad code in there?
  • Is there anything I've made more complicated than I needed to?
  • You can see I used echo "<br />" in there, a friend told me that echo "\n" should work, but it isn't.
  • Any other comments you may have on either my code or my method.

<html>
<head><title>Fibonacci Sequence</title></head>

<body>
<form action="fib.php" method="post"> 
Quantity: <input name="limit" type="text" /> 
<input type="submit" />
</form>

<?php

if(!empty($_POST)){

$lim = $_POST['limit'];
$fst = 0;
$snd = 1;
$nxt = $fst + $snd;
echo "The limit you provided was " . $lim . ".<br />The sequence is: " . $fst . ", " . $snd;

while ($nxt <= $lim){
echo ", " . $nxt;
$fst = $snd;
$snd = $nxt;
$nxt = $fst + $snd;
}
}
?>

</body>
</html>



Thanks.

JD

This post has been edited by JoshD: 16 August 2011 - 06:42 AM


Is This A Good Question/Topic? 0
  • +

Replies To: Feedback on my first PHP script (fibonacci sequence)

#2 Dormilich  Icon User is offline

  • 痛覚残留
  • member icon

Reputation: 3572
  • View blog
  • Posts: 10,414
  • Joined: 08-June 10

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 16 August 2011 - 07:32 AM

<br />: youíre not using XHTML, thus using the self-closing tags (/>) are invalid. the correct usage still is <br>. In HTML text, a line break (\n) is converted to a single space. exception: text in a <textarea> element.

other comments:
  • the DTD/Doctype is missing
  • you should separate your HTML and PHP (hardly mentioned in any beginner tutorial, though). you can fill threads about the why Ö
  • if(!empty($_POST)) not a reliable way to determine a POSTed form. better is using $_SERVER['REQUEST_METHOD'] or (even better) using PHP filter functions. those will return false if the parameter is not present.
  • line #14: throws a warning if 'limit' is not given. requires at least a test with isset() or the use of the filter functions.
  • you can (and IMO should) not redeclare variables, if it is not necessary ($lim = $_POST['limit']). this only hides their origin which often leads to "forgotten security" issues.
  • use functions. they make life easier. really.

Was This Post Helpful? 3
  • +
  • -

#3 Dormilich  Icon User is offline

  • 痛覚残留
  • member icon

Reputation: 3572
  • View blog
  • Posts: 10,414
  • Joined: 08-June 10

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 16 August 2011 - 08:04 AM

just for your information, some advanced way of doing it:
//# can be loaded through an autoloader
class FibonacciIterator implements Iterator
{
    private 
          $summand_1 = 0
        , $summand_2 = 1
        , $limit     = 1
    ;
    const
          START_VALUE_1 = 0
        , START_VALUE_2 = 1
    ;
    public function __construct(
        $limit = 1
    )
    {
        $this->limit = (int) $limit;
    }
    
    public function rewind()
    {
        $this->summand_1 = self::START_VALUE_1;
        $this->summand_2 = self::START_VALUE_2;
    }
    
    public function current()
    {
        return ($this->summand_1 + $this->summand_2);
    }
    
    public function key()
    {
        return $this->summand_2;
    }
    
    public function next()
    {
        $temp             = $this->summand_1;
        $this->summand_1  = $this->summand_2;
        $this->summand_2 += $temp;
    }
    
    public function valid()
    {
        return ($this->summand_1 + $this->summand_2 <= $this->limit);
    }
}


$fit =  new FibonacciIterator(1000);
$sequence = FibonacciIterator::START_VALUE_1 . '<br>'
          . FibonacciIterator::START_VALUE_2 . '<br>';
foreach ($fit as $fib_number)
{
    $sequence .= $fib_number . '<br>';
}
echo $sequence; //# resp. replace in your HTML template

This post has been edited by Dormilich: 16 August 2011 - 12:29 PM
Reason for edit:: optimizing the class

Was This Post Helpful? 1
  • +
  • -

#4 JoshD  Icon User is offline

  • D.I.C Head

Reputation: 9
  • View blog
  • Posts: 111
  • Joined: 22-March 10

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 16 August 2011 - 08:25 AM

View PostDormilich, on 16 August 2011 - 02:32 PM, said:

<br />: youíre not using XHTML, thus using the self-closing tags (/>) are invalid. the correct usage still is <br>. In HTML text, a line break (\n) is converted to a single space. exception: text in a <textarea> element.
Ah yes, I know that one, I literally just through the html head and body tags on to test wether the PHP worked. But thanks.

View PostDormilich, on 16 August 2011 - 02:32 PM, said:

other comments:
  • the DTD/Doctype is missing
Same as above.

View PostDormilich, on 16 August 2011 - 02:32 PM, said:

  • you should separate your HTML and PHP (hardly mentioned in any beginner tutorial, though). you can fill threads about the why Ö
  • if(!empty($_POST)) not a reliable way to determine a POSTed form. better is using $_SERVER['REQUEST_METHOD'] or (even better) using PHP filter functions. those will return false if the parameter is not present.
  • line #14: throws a warning if 'limit' is not given. requires at least a test with isset() or the use of the filter functions.
  • you can (and IMO should) not redeclare variables, if it is not necessary ($lim = $_POST['limit']). this only hides their origin which often leads to "forgotten security" issues.
  • use functions. they make life easier. really.

All these have been helpful.
Also, I've been reading through some things on classes and functions and would like to re-write this as one soon.
And I will look at the other thing you've posted when I get home from work. Thanks.

JD
Was This Post Helpful? 0
  • +
  • -

#5 JoshD  Icon User is offline

  • D.I.C Head

Reputation: 9
  • View blog
  • Posts: 111
  • Joined: 22-March 10

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 17 August 2011 - 02:28 AM

View PostDormilich, on 16 August 2011 - 02:32 PM, said:

other comments:
  • if(!empty($_POST)) not a reliable way to determine a POSTed form. better is using $_SERVER['REQUEST_METHOD'] or (even better) using PHP filter functions. those will return false if the parameter is not present.
  • line #14: throws a warning if 'limit' is not given. requires at least a test with isset() or the use of the filter functions.
  • you can (and IMO should) not redeclare variables, if it is not necessary ($lim = $_POST['limit']). this only hides their origin which often leads to "forgotten security" issues.
  • use functions. they make life easier. really.


Okay, I was looking at filter functions... i'm not really seeing which one can check if the form is empty or not.
I've changed it so I'm not redeclaring variables.
And I will re-write it as a function after I understand these over 2 things.

Thanks.

JD
Was This Post Helpful? 0
  • +
  • -

#6 Dormilich  Icon User is offline

  • 痛覚残留
  • member icon

Reputation: 3572
  • View blog
  • Posts: 10,414
  • Joined: 08-June 10

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 17 August 2011 - 03:03 AM

View PostJoshD, on 17 August 2011 - 11:28 AM, said:

Okay, I was looking at filter functions... i'm not really seeing which one can check if the form is empty or not.

filter functions donít check, whether the form was submitted, they check for individual form values.

for example, in your script, you only need the value "limit". either you have that value, or you donít. if not, then you probably donít have the form submitted. but the reason why "limit" is not present doesnít matter (esp. in combination with content-code separation)
//# $limit will be an interger ≥ 1 
//# or false (validation failed)
//# or NULL (form not submitted)
$limit = filter_input(INPUT_POST, "limit", FILTER_VALIDATE_INT, array("options" => array("min_range" => 1)));

//# check if there is a valid value
if (!is_int($limit))
{
    throw new OutOfBoundsException("Limit is invalid.");
}

//# proceed with script

Was This Post Helpful? 1
  • +
  • -

#7 JoshD  Icon User is offline

  • D.I.C Head

Reputation: 9
  • View blog
  • Posts: 111
  • Joined: 22-March 10

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 17 August 2011 - 05:14 AM

View PostDormilich, on 17 August 2011 - 10:03 AM, said:

View PostJoshD, on 17 August 2011 - 11:28 AM, said:

Okay, I was looking at filter functions... i'm not really seeing which one can check if the form is empty or not.

filter functions don’t check, whether the form was submitted, they check for individual form values.

for example, in your script, you only need the value "limit". either you have that value, or you don’t. if not, then you probably don’t have the form submitted. but the reason why "limit" is not present doesn’t matter (esp. in combination with content-code separation)
//# $limit will be an interger ≥ 1 
//# or false (validation failed)
//# or NULL (form not submitted)
$limit = filter_input(INPUT_POST, "limit", FILTER_VALIDATE_INT, array("options" => array("min_range" => 1)));

//# check if there is a valid value
if (!is_int($limit))
{
    throw new OutOfBoundsException("Limit is invalid.");
}

//# proceed with script


Couldn't I achieve the same result by doing?
if($_POST['limit'] >= 2) {


This post has been edited by JoshD: 17 August 2011 - 05:14 AM

Was This Post Helpful? 0
  • +
  • -

#8 RudiVisser  Icon User is offline

  • .. does not guess solutions
  • member icon

Reputation: 1004
  • View blog
  • Posts: 3,562
  • Joined: 05-June 09

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 17 August 2011 - 05:20 AM

No.

You could with this:
if (!empty($_POST['limit']) && $_POST['limit'] >= 2) {

Was This Post Helpful? 0
  • +
  • -

#9 JoshD  Icon User is offline

  • D.I.C Head

Reputation: 9
  • View blog
  • Posts: 111
  • Joined: 22-March 10

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 17 August 2011 - 05:27 AM

View PostRudiVisser, on 17 August 2011 - 12:20 PM, said:

No.

You could with this:
if (!empty($_POST['limit']) && $_POST['limit'] >= 2) {


1) I was just told !empty wasn't a good way to determine POSTed forms.
2) If the input value has to be greater than 2 and an empty form what = 0 surely it achieves the same. Or am I misunderstanding what !empty does?
Was This Post Helpful? 0
  • +
  • -

#10 RudiVisser  Icon User is offline

  • .. does not guess solutions
  • member icon

Reputation: 1004
  • View blog
  • Posts: 3,562
  • Joined: 05-June 09

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 17 August 2011 - 05:32 AM

[quote name='JoshD' date='17 August 2011 - 01:27 PM' timestamp='1313584062' post='1413847']

View PostRudiVisser, on 17 August 2011 - 12:20 PM, said:

1) I was just told !empty wasn't a good way to determine POSTed forms.
2) If the input value has to be greater than 2 and an empty form what = 0 surely it achieves the same. Or am I misunderstanding what !empty does?

!empty($_POST) isn't necessarily a good way. When I first started I always did !empty($_POST['submit']) (where submit was the submit button) until I realised that in some browsers that doesn't come through if you hit Enter.

Remember that in some cases checking the request method is not enough. An example would be a page with multiple forms that all need processing differently. At this point, it's probably best to use a hidden field per form and define which form it is, then detect that your form has being submitted based on that element. You can even have a common element with a different value on it across forms. Do a switch based on that field after checking $_SERVER['REQUEST_METHOD'] for POST.

2; If $_POST['limit'] was 0 (or not set), empty would return true, flipping that with ! makes it return false which means it wouldn't run what is inside the { .. }s.

This post has been edited by RudiVisser: 17 August 2011 - 05:42 AM

Was This Post Helpful? 0
  • +
  • -

#11 Dormilich  Icon User is offline

  • 痛覚残留
  • member icon

Reputation: 3572
  • View blog
  • Posts: 10,414
  • Joined: 08-June 10

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 17 August 2011 - 06:32 AM

View PostRudiVisser, on 17 August 2011 - 02:32 PM, said:

Remember that in some cases checking the request method is not enough. An example would be a page with multiple forms that all need processing differently.

but then I would submit each form to a different script (URI).
Was This Post Helpful? 0
  • +
  • -

#12 JackOfAllTrades  Icon User is offline

  • Saucy!
  • member icon

Reputation: 6092
  • View blog
  • Posts: 23,612
  • Joined: 23-August 08

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 17 August 2011 - 06:48 AM

Quote

<br />: youíre not using XHTML, thus using the self-closing tags (/>) are invalid.


Wow, I actually did not know that! Whoops!
Was This Post Helpful? 0
  • +
  • -

#13 codeprada  Icon User is offline

  • Changed Man With Different Priorities
  • member icon

Reputation: 948
  • View blog
  • Posts: 2,357
  • Joined: 15-February 11

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 17 August 2011 - 07:16 AM

View PostJackOfAllTrades, on 17 August 2011 - 09:48 AM, said:

Quote

<br />: youíre not using XHTML, thus using the self-closing tags (/>) are invalid.


Wow, I actually did not know that! Whoops!

Same here... So what about
<img src="..." alt="" />
I'm guessing it should be
<img src="..." alt="">

Was This Post Helpful? 0
  • +
  • -

#14 Dormilich  Icon User is offline

  • 痛覚残留
  • member icon

Reputation: 3572
  • View blog
  • Posts: 10,414
  • Joined: 08-June 10

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 17 August 2011 - 07:32 AM

yepp.

if you’re actually using XHTML, you don’t need to worry about HTML errors … you will be prompted for every single one.

the only time when you should use the XHTML syntax in HTML is when you also need to load it into an XML parser (for whatever reason).

This post has been edited by Dormilich: 17 August 2011 - 07:36 AM

Was This Post Helpful? 0
  • +
  • -

#15 CTphpnwb  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 3077
  • View blog
  • Posts: 10,790
  • Joined: 08-August 08

Re: Feedback on my first PHP script (fibonacci sequence)

Posted 17 August 2011 - 08:16 AM

I like to use an array of expected inputs so that I can be sure that I use all of them and only them. I'd do something like:
<?php
$expected_posts = array("name"=>"string", "address1"=>"string", "address2"=>"string", "city"=>"string", "state"=>"string", "zip"=>"int");
$submitted_values = array();
foreach($expected_posts as $key => $value) {
	$error = checkvals($key, $value);
	if($error != "") {
		$errors[$key] = $error;
	}
}
if(count($errors) > 0) {
	// Code to display form again with error message(s).
} else {
	// Code to display results of submitting the form.
}


function checkvals($item, $type) {
	if(empty($_POST[$item])) {
		return "is Empty!";
	}
	switch($type) {
		case "string":
			if(strlen($_POST[$item]) == 0) {
				return "is Empty!";
			}
			break;
		case "int":
			if((int)$_POST[$item] == 0) {
				return "is not a valid number.";
			}
			break;
	}
	return "";
}

It's untested so it probably has bugs, but it should be close.
Was This Post Helpful? 0
  • +
  • -

  • (2 Pages)
  • +
  • 1
  • 2