What is wrong in your code?
Simply put: everything.
How to fix it
Let's analyze each piece of it and see how to correct and improve it.
class GuestBook {
public function __construct($path)
{
$this->path = $path;
$this->list = $list;
}
The first statement of the constructor is correct, it does what the constructor should do: it initializes the class members. It would be nice for the readers of the code to also declare $path and $list on top of the class definition like this:
class GuessBook {
private $path;
private $list;
public function __construct($path)
...
The second statement of the constructor, however, initializes the list property of the object ($this->list) with the value of variable $list that is not mentioned anywhere in the function; it doesn't exist and that means it is undefined and its value is NULL.
You can either pass $list as the second argument of the constructor (public function __construct($path, $list)) or initialize $this->list with a fixed value. From the way you use the class I think the second option is the way to go and the most appropriate value for $this->list is an empty array().
public function getData()
{
$list = file($this->path);
return $this->list;
}
The first statement loads the content of a file, splits it into lines and stores the array of lines into the local variable $list. Nothing wrong here. But the second statement returns the value of the list property of the current object that, let's remember, was initialized in the constructor with NULL.
I believe your intention is to store in the list property the array of lines loaded from the file. That is achieved by changing the first line of code to:
$this->list = file($this->path);
While we are here, the name of the method doesn't clearly represent what it does. Its main purpose is to load the content of the file in $this->list. A better name for it is loadData().
public function append($text)
{
$list = file($this->path);
$list[] = $text;
var_dump($list);
return $this->list;
}
First of all, the var_dump() has no place here (or everywhere). It is a debugging function, one puts it into the code, uses the data it displays and removes it from the code as soon as the code was fixed and its presence is not needed any more. I hope you let it there in the code posted here and you remove it from the real code.
This method has the same problem as getData(). It loads the data into the local variable $list that is destroyed as soon as the function completes. However, using $this->list instead of $list fixes only half of the problem.
What happens when you call append() twice in a row (after you have fixed it)? The second call discards the content of $this->list prepared by the first call. This (partially) happens because append() does two things: loads the data and appends a new line. Let getData() load the data and change append() to only append one new line:
public function append($text)
{
$this->list[] = $text;
return $this->list;
}
public function getList()
{
var_dump($list); // NULL
}
The function name (getList()) says it returns a value. Instead it displays something. This is confusing. Again, there is no place for var_dump() here.
But the main problem of this function is that it attempts to use the local variable $list that is not initialized. It should probably read:
public function getList()
{
return $this->list;
}
};
There is no need to put a semicolon (;) after a class definition. It is not an error, though. It is just an empty statement.
$newBook = new GuestBook(__DIR__ . '/HW_db.txt');
$newBook->getData();
$newBook->append('HI!');
$newBook->getList(); // NULL??
Nothing wrong here, just unexpected. A reader that doesn't know the code of class GuestBook expects $newBook->getList() returns a value and not print anything.
var_dump($newBook->list); // NULL ??
$newBook->list is NULL because the one and only time when something was stored in it is in the constructor. And the value put in it by the constructor is the value of an uninitialized variable, i.e. NULL.
More than that, the property list is accessible from outside the class because you didn't declare it and PHP created it as public the first time when it was set. In a correct OOP code, most of the times the properties are private or protected, only (some of) the methods are public.
How to make it work
Let's fix and put everything together:
class GuestBook {
private $path;
private $list;
public function __construct($path)
{
$this->path = $path;
$this->list = array();
}
public function loadData()
{
$this->list = file($this->path);
return $this->list;
}
public function append($text)
{
$this->list[] = $text;
return $this->list;
}
public function getList()
{
return $this->list;
}
}
$newBook = new GuestBook(__DIR__ . '/HW_db.txt');
$newBook->loadData();
$newBook->append('HI!');
print_r($newBook->getList());
While this code works, it still shows several weak points. If you call loadData() again after append(), the previously appended data is discarded and the content of the file is loaded again. If you don't call loadData() at all, the content of the file is ignored.
These flaws can be easily fixed by merging the code of loadData() into the constructor and removing the method loadData() altogether. This way the class goes one step further towards a correct OOP implementation. The users of the class does not and should not care about loading or saving the data; this is an implementation detail and it's the responsibility of the class.
Also, you need a method to put the data back into the file. This can be done in the destructor of the class.
class GuestBook {
private $path;
private $list;
public function __construct($path)
{
$this->path = $path;
$this->list = file($this->path);
}
public function append($text)
{
$this->list[] = $text;
return $this->list;
}
public function getList()
{
return $this->list;
}
public function __destruct()
{
file_put_contents(this->file, implode('', $this->list));
}
}
$newBook = new GuestBook(__DIR__ . '/HW_db.txt');
$newBook->append('HI!');
print_r($newBook->getList());
unset($newBook); // This calls the destructor
The code still has a flaw: file() loads the content of the file, splits it into lines but each line ends with the end-of-line character ("\n"). When the data is saved back into the file, the lines are joined together. If, because of append(), the list contains entries that do not end with "\n" or entries that contain embedded newlines, the next time when the data is loaded from file the content of $this->list will be different.
My suggestion is to always make sure the elements of $this->list do not contain new line characters.
Change the constructor to remove the newlines and the destructor to put them back when it saves the data in the file. Also change append() to replace the newline characters with spaces.
The final code
class GuestBook {
private $path;
private $list;
public function __construct($path)
{
$this->path = $path;
$this->list = explode("\n", file_get_contents($this->path));
}
public function append($text)
{
$this->list[] = trim(str_replace("\n", ' ', $text));
return $this->list;
}
public function getList()
{
return $this->list;
}
public function __destruct()
{
file_put_contents(this->file, implode("\n", $this->list));
}
}
What this code doesn't do and it must do
The code above works fine in an ideal world where all the files exist and are readable and writable and nothing bad ever happens. Since we are living in the real world and not in a fairy tale, bad things sometimes happen and the code must be ready to handle them.
For the sake of simplicity, the code above doesn't handle the errors: file doesn't exist, it cannot be read or written. This can be easily accomplished by using the function file_exists() and by checking the values returned by file_get_contents() and file_put_contents().
Further reading
Read the entire section about PHP classes and objects. It doesn't replace a good introduction into OOP, though. Find and read one if you feel you need (after or while you read the PHP documentation).
The documentation of PHP functions used in the code: file(), file_get_contents(), file_put_contents(), trim(), str_replace(), explode(), implode().
Also read about the scope of variables in PHP.
$listvariable, where did you defined it?__construct($path)you're only using one argument here and 2 after. So$this->list = $list;will not pass.