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.
1 Answer 1
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
varkeyword 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) {
You must log in to answer this question.
Explore related questions
See similar questions with these tags.