From c5f0c1aa8e8b2c49dc2daf05b4f78c8aeb4d9388 Mon Sep 17 00:00:00 2001
From: Andreas Fischer <bantu@phpbb.com>
Date: Fri, 8 Aug 2014 14:43:57 +0200
Subject: [ticket/12937] Do not return false. Return null for get and array()
 for all.

PHPBB3-12937
---
 phpBB/phpbb/config_php_file.php | 39 ++++++++++++---------------------------
 tests/config_php_file_test.php  | 10 ++++++++++
 2 files changed, 22 insertions(+), 27 deletions(-)

diff --git a/phpBB/phpbb/config_php_file.php b/phpBB/phpbb/config_php_file.php
index 1a562e470d..7445e7df22 100644
--- a/phpBB/phpbb/config_php_file.php
+++ b/phpBB/phpbb/config_php_file.php
@@ -71,59 +71,44 @@ class config_php_file
 	/**
 	* Returns an associative array containing the variables defined by the config file.
 	*
-	* @return bool|array Return the content of the config file or false if the file does not exists.
+	* @return array Return the content of the config file or an empty array if the file does not exists.
 	*/
 	public function get_all()
 	{
-		if (!$this->load_config_file())
-		{
-			return false;
-		}
+		$this->load_config_file();
 
 		return $this->config_data;
 	}
 
 	/**
-	* Return the value of a variable defined into the config.php file and false if the variable does not exist.
+	* Return the value of a variable defined into the config.php file or null if the variable does not exist.
 	*
 	* @param string $variable The name of the variable
-	* @return mixed
+	* @return mixed Value of the variable or null if the variable is not defined.
 	*/
 	public function get($variable)
 	{
-		if (!$this->load_config_file())
-		{
-			return false;
-		}
+		$this->load_config_file();
 
-		return isset($this->config_data[$variable]) ? $this->config_data[$variable] : false;
+		return isset($this->config_data[$variable]) ? $this->config_data[$variable] : null;
 	}
 
 	/**
 	* Load the config file and store the information.
 	*
-	* @return bool True if the file was correctly loaded, false otherwise.
+	* @return null
 	*/
 	protected function load_config_file()
 	{
-		if (!$this->config_loaded)
+		if (!$this->config_loaded && file_exists($this->config_file))
 		{
-			if (file_exists($this->config_file))
-			{
-				$this->defined_vars = get_defined_vars();
+			$this->defined_vars = get_defined_vars();
 
-				require($this->config_file);
-				$this->config_data = array_diff_key(get_defined_vars(), $this->defined_vars);
+			require($this->config_file);
+			$this->config_data = array_diff_key(get_defined_vars(), $this->defined_vars);
 
-				$this->config_loaded = true;
-			}
-			else
-			{
-				return false;
-			}
+			$this->config_loaded = true;
 		}
-
-		return true;
 	}
 
 	/**
diff --git a/tests/config_php_file_test.php b/tests/config_php_file_test.php
index c2e4eb21c7..c319678108 100644
--- a/tests/config_php_file_test.php
+++ b/tests/config_php_file_test.php
@@ -17,6 +17,7 @@ class phpbb_config_php_file_test extends phpbb_test_case
 	{
 		$config_php = new \phpbb\config_php_file(dirname( __FILE__ ) . '/fixtures/', 'php');
 		$this->assertSame('bar', $config_php->get('foo'));
+		$this->assertNull($config_php->get('bar'));
 		$this->assertSame(array('foo' => 'bar', 'foo_foo' => 'bar bar'), $config_php->get_all());
 	}
 
@@ -25,6 +26,15 @@ class phpbb_config_php_file_test extends phpbb_test_case
 		$config_php = new \phpbb\config_php_file(dirname( __FILE__ ) . '/fixtures/', 'php');
 		$config_php->set_config_file(dirname( __FILE__ ) . '/fixtures/config_other.php');
 		$this->assertSame('foo', $config_php->get('bar'));
+		$this->assertNull($config_php->get('foo'));
 		$this->assertSame(array('bar' => 'foo', 'bar_bar' => 'foo foo'), $config_php->get_all());
 	}
+
+	public function test_non_existent_file()
+	{
+		$config_php = new \phpbb\config_php_file(dirname( __FILE__ ) . '/fixtures/non_existent/', 'php');
+		$this->assertNull($config_php->get('bar'));
+		$this->assertNull($config_php->get('foo'));
+		$this->assertSame(array(), $config_php->get_all());
+	}
 }
-- 
cgit v1.2.1