Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Add chaining ability to JTable #1154

Closed
wants to merge 2 commits into from

2 participants

Nikolai Plath Andrew Eddie
Nikolai Plath

No description provided.

Andrew Eddie eddieajau commented on the diff
libraries/joomla/table/table.php
@@ -489,7 +493,7 @@ public function load($keys = null, $reset = true)
// If empty primary key there's is no need to load anything
if (empty($keyValue))
{
- return true;

There's still a return false in load, and the DocBlock would need to be changed. Is there a risk the chaining here will cause a false positive in terms of backward compatibility?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Andrew Eddie eddieajau commented on the diff
libraries/joomla/table/table.php
((6 lines not shown))
*
* @link http://docs.joomla.org/JTable/check
* @since 11.1
*/
public function check()
{
- return true;

Check is often called and return value checked. I think this might cause a few problems ... you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Andrew Eddie eddieajau commented on the diff
libraries/joomla/table/table.php
@@ -590,7 +594,7 @@ public function store($updateNulls = false)
// If the table is not set to track assets return true.
if (!$this->_trackAssets)
{
- return true;

Store has quite a few return false statements so I'm not sure we can chain it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Andrew Eddie eddieajau commented on the diff
libraries/joomla/table/table.php
@@ -713,7 +717,7 @@ public function save($src, $orderingFilter = '', $ignore = '')
// Set the error to empty and return true.
$this->setError('');
- return true;

Same in the save method. return false and other methods like delete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Nikolai Plath elkuku closed this
Nikolai Plath

another day..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 15, 2012
  1. Nikolai Plath

    Add chaining ability to JTable

    elkuku authored
  2. Nikolai Plath

    Update a unit test

    elkuku authored
This page is out of date. Refresh to see the latest.
86 libraries/joomla/table/table.php
View
@@ -345,7 +345,7 @@ public function getDbo()
*
* @param JDatabaseDriver $db A JDatabaseDriver object to be used by the table object.
*
- * @return boolean True on success.
+ * @return JTable.
*
* @link http://docs.joomla.org/JTable/setDBO
* @since 11.1
@@ -354,7 +354,7 @@ public function setDBO(JDatabaseDriver $db)
{
$this->_db = $db;
- return true;
+ return $this;
}
/**
@@ -362,7 +362,7 @@ public function setDBO(JDatabaseDriver $db)
*
* @param mixed $input A JAccessRules object, JSON string, or array.
*
- * @return void
+ * @return JTable
*
* @since 11.1
*/
@@ -376,6 +376,8 @@ public function setRules($input)
{
$this->_rules = new JAccessRules($input);
}
+
+ return $this;
}
/**
@@ -395,7 +397,7 @@ public function getRules()
* definition. It will ignore the primary key as well as any private class
* properties.
*
- * @return void
+ * @return JTable
*
* @link http://docs.joomla.org/JTable/reset
* @since 11.1
@@ -411,6 +413,8 @@ public function reset()
$this->$k = $v->Default;
}
}
+
+ return $this;
}
/**
@@ -421,11 +425,11 @@ public function reset()
* @param mixed $src An associative array or object to bind to the JTable instance.
* @param mixed $ignore An optional array or space separated list of properties to ignore while binding.
*
- * @return boolean True on success.
+ * @return JTable.
*
* @link http://docs.joomla.org/JTable/bind
* @since 11.1
- * @throws UnexpectedValueException
+ * @throws InvalidArgumentException
*/
public function bind($src, $ignore = array())
{
@@ -460,7 +464,7 @@ public function bind($src, $ignore = array())
}
}
- return true;
+ return $this;
}
/**
@@ -489,7 +493,7 @@ public function load($keys = null, $reset = true)
// If empty primary key there's is no need to load anything
if (empty($keyValue))
{
- return true;

There's still a return false in load, and the DocBlock would need to be changed. Is there a risk the chaining here will cause a false positive in terms of backward compatibility?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ return $this;
}
$keys = array($keyName => $keyValue);
@@ -542,14 +546,14 @@ public function load($keys = null, $reset = true)
* method to make sure the data they are storing in the database is safe and
* as expected before storage.
*
- * @return boolean True if the instance is sane and able to be stored in the database.
+ * @return JTable If the instance is sane and able to be stored in the database.
*
* @link http://docs.joomla.org/JTable/check
* @since 11.1
*/
public function check()
{
- return true;

Check is often called and return value checked. I think this might cause a few problems ... you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ return $this;
}
/**
@@ -561,7 +565,7 @@ public function check()
*
* @param boolean $updateNulls True to update fields even if they are null.
*
- * @return boolean True on success.
+ * @return JTable
*
* @link http://docs.joomla.org/JTable/store
* @since 11.1
@@ -590,7 +594,7 @@ public function store($updateNulls = false)
// If the table is not set to track assets return true.
if (!$this->_trackAssets)
{
- return true;

Store has quite a few return false statements so I'm not sure we can chain it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ return $this;
}
if ($this->_locked)
@@ -656,7 +660,7 @@ public function store($updateNulls = false)
$this->_db->execute();
}
- return true;
+ return $this;
}
/**
@@ -672,7 +676,7 @@ public function store($updateNulls = false)
* @param mixed $ignore An optional array or space separated list of properties
* to ignore while binding.
*
- * @return boolean True on success.
+ * @return JTable
*
* @link http://docs.joomla.org/JTable/save
* @since 11.1
@@ -713,7 +717,7 @@ public function save($src, $orderingFilter = '', $ignore = '')
// Set the error to empty and return true.
$this->setError('');
- return true;

Same in the save method. return false and other methods like delete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ return $this;
}
/**
@@ -721,7 +725,7 @@ public function save($src, $orderingFilter = '', $ignore = '')
*
* @param mixed $pk An optional primary key value to delete. If not set the instance property value is used.
*
- * @return boolean True on success.
+ * @return JTable
*
* @link http://docs.joomla.org/JTable/delete
* @since 11.1
@@ -772,7 +776,7 @@ public function delete($pk = null)
// Check for a database error.
$this->_db->execute();
- return true;
+ return $this;
}
/**
@@ -785,19 +789,20 @@ public function delete($pk = null)
*
* @param integer $userId The Id of the user checking out the row.
* @param mixed $pk An optional primary key value to check out. If not set
- * the instance property value is used.
+ * the instance property value is used.
*
- * @return boolean True on success.
+ * @return JTable
*
* @link http://docs.joomla.org/JTable/checkOut
* @since 11.1
+ * @throws UnexpectedValueException
*/
public function checkOut($userId, $pk = null)
{
// If there is no checked_out or checked_out_time field, just return true.
if (!property_exists($this, 'checked_out') || !property_exists($this, 'checked_out_time'))
{
- return true;
+ return $this;
}
// Initialise variables.
@@ -826,7 +831,7 @@ public function checkOut($userId, $pk = null)
$this->checked_out = (int) $userId;
$this->checked_out_time = $time;
- return true;
+ return $this;
}
/**
@@ -835,17 +840,18 @@ public function checkOut($userId, $pk = null)
*
* @param mixed $pk An optional primary key value to check out. If not set the instance property value is used.
*
- * @return boolean True on success.
+ * @return JTable
*
* @link http://docs.joomla.org/JTable/checkIn
* @since 11.1
+ * @throws UnexpectedValueException
*/
public function checkIn($pk = null)
{
// If there is no checked_out or checked_out_time field, just return true.
if (!property_exists($this, 'checked_out') || !property_exists($this, 'checked_out_time'))
{
- return true;
+ return $this;
}
// Initialise variables.
@@ -873,7 +879,7 @@ public function checkIn($pk = null)
$this->checked_out = 0;
$this->checked_out_time = '';
- return true;
+ return $this;
}
/**
@@ -881,7 +887,7 @@ public function checkIn($pk = null)
*
* @param mixed $pk An optional primary key value to increment. If not set the instance property value is used.
*
- * @return boolean True on success.
+ * @return JTable
*
* @link http://docs.joomla.org/JTable/hit
* @since 11.1
@@ -891,7 +897,7 @@ public function hit($pk = null)
// If there is no hits field, just return true.
if (!property_exists($this, 'hits'))
{
- return true;
+ return $this;
}
// Initialise variables.
@@ -915,7 +921,7 @@ public function hit($pk = null)
// Set table values in the object.
$this->hits++;
- return true;
+ return $this;
}
/**
@@ -965,6 +971,7 @@ public function isCheckedOut($with = 0, $against = null)
*
* @link http://docs.joomla.org/JTable/getNextOrder
* @since 11.1
+ * @throws UnexpectedValueException
*/
public function getNextOrder($where = '')
{
@@ -997,10 +1004,11 @@ public function getNextOrder($where = '')
*
* @param string $where WHERE clause to use for limiting the selection of rows to compact the ordering values.
*
- * @return mixed Boolean true on success.
+ * @return JTable
*
* @link http://docs.joomla.org/JTable/reorder
* @since 11.1
+ * @throws UnexpectedValueException
*/
public function reorder($where = '')
{
@@ -1008,8 +1016,6 @@ public function reorder($where = '')
if (!property_exists($this, 'ordering'))
{
throw new UnexpectedValueException(sprintf('%s does not support ordering.', get_class($this)));
- $this->setError($e);
- return false;
}
// Initialise variables.
@@ -1051,7 +1057,7 @@ public function reorder($where = '')
}
}
- return true;
+ return $this;
}
/**
@@ -1062,7 +1068,7 @@ public function reorder($where = '')
* @param string $where WHERE clause to use for limiting the selection of rows to compact the
* ordering values.
*
- * @return mixed Boolean true on success.
+ * @return JTable
*
* @link http://docs.joomla.org/JTable/move
* @since 11.1
@@ -1079,7 +1085,7 @@ public function move($delta, $where = '')
// If the change is none, do nothing.
if (empty($delta))
{
- return true;
+ return $this;
}
// Initialise variables.
@@ -1147,7 +1153,7 @@ public function move($delta, $where = '')
$this->_db->execute();
}
- return true;
+ return $this;
}
/**
@@ -1159,7 +1165,7 @@ public function move($delta, $where = '')
* @param integer $state The publishing state. eg. [0 = unpublished, 1 = published]
* @param integer $userId The user id of the user performing the operation.
*
- * @return boolean True on success; false if $pks is empty.
+ * @return mixed JTable on success; false if $pks is empty.
*
* @link http://docs.joomla.org/JTable/publish
* @since 11.1
@@ -1227,13 +1233,13 @@ public function publish($pks = null, $state = 1, $userId = 0)
}
$this->setError('');
- return true;
+ return $this;
}
/**
* Method to lock the database table for writing.
*
- * @return boolean True on success.
+ * @return JTable
*
* @since 11.1
* @throws RuntimeException
@@ -1243,13 +1249,13 @@ protected function _lock()
$this->_db->lockTable($this->_tbl);
$this->_locked = true;
- return true;
+ return $this;
}
/**
* Method to unlock the database table for writing.
*
- * @return boolean True on success.
+ * @return JTable
*
* @since 11.1
*/
@@ -1258,6 +1264,6 @@ protected function _unlock()
$this->_db->unlockTables();
$this->_locked = false;
- return true;
+ return $this;
}
}
2  tests/suites/unit/joomla/table/JTableLanguageTest.php
View
@@ -112,7 +112,7 @@ public function testStore()
$table->lang_code = 'en-GB';
$this->assertFalse($table->store(), 'Line: ' . __LINE__ . ' Table store should fail due to a duplicated lang_code field.');
$table->lang_code = 'en-US';
- $this->assertTrue($table->store(), 'Line: ' . __LINE__ . ' Table store should successfully insert a record for English (US).');
+ $this->assertInstanceOf('JTable', $table->store(), 'Line: ' . __LINE__ . ' Table store should successfully insert a record for English (US).');
}
}
Something went wrong with that request. Please try again.