3
\$\begingroup\$

I am learning PHP 7 and decided to implement a simple FIFO singly-linked queue in it:

<?php

class QueueNode {
    var $item;
    var $next;

    function __construct($item) {
        $this->item = $item;
    }

    function get_item() {
        return $this->item;
    }
}

class Queue {
    var $head;
    var $tail;
    var $size;

    function push($item) {
        if ($this->size == 0) {
            $this->head = $this->tail = new QueueNode($item);
        } else {
            $new_node = new QueueNode($item);
            $this->tail->next = $new_node;
            $this->tail = $new_node;
        }

        $this->size++;
    }

    function pop() {
        if ($this->size == 0) {
            throw new Exception("Popping from an empty queue.");
        }

        $ret = $this->head->item;
        $this->head = $this->head->next;

        if (--$this->size == 0) {
            $this->head = $this->tail = null;
        }

        return $ret;
    }
}

$queue = new Queue();

for ($i = 1; $i <= 10; $i++) {
    $queue->push($i);
}

for ($i = 1; $i <= 10; $i++) {
    echo $queue->pop() . " ";
}

echo "<br>";
?>

As always, any critique is much appreciated.

\$\endgroup\$
3
\$\begingroup\$

Getters and setters

So there is a getter for item (i.e. get_item()) - why not a getter (and setter) for the next property? If getters and setters are added, then those properties could be set to private visibility if deemed appropriate.

Are you familiar with the PHP Standards Recommendations? While it is only a suggestion, you might find them helpful for writing code that is maintainable and readable, not only for others but also your future self.

According to PSR-1:

Method names MUST be declared in camelCase().1

A format of getItem() is more widely used amongst many PHP developers.

Types and conditions

Inside Queue::push() and Queue::pop() I see this common first line:

if ($this->size == 0) {

It is advisable to use strict equality comparison operators (i.e. === and !==) to avoid excess type-juggling. As a consequence, size should perhaps be declared with a initial value of 0.

The conditional could also be shortened to if (!$this->size) { though that would allow any type that is considered to FALSE to allow the conditional block to be executed.

keyword var

var is a feature from PHP 4 and it isn't needed anymore2 - while it works as a synonym for public it is deprecated. If you adhere to PSR-2 then note it specifies:

The var keyword MUST NOT be used to declare a property.3

for loops

There isn't anything wrong with a traditional loop, but you could use the range() function to iterate $i for you in a foreach loop:

for ($i = 1; $i <= 10; $i++) {

can be changed to:

foreach(range(1, 10) as $i) {
\$\endgroup\$
0

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.