Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Removed "_" prefix from private and protected attributes. #418

Closed
wants to merge 1 commit into from

4 participants

@kimhemsoe
Collaborator

No description provided.

@doctrinebot
Collaborator

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DBAL-671

We use Jira to track the state of pull requests and the versions they got
included in.

@stof stof commented on the diff
lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php
@@ -41,27 +41,27 @@ class MysqliStatement implements \IteratorAggregate, Statement
/**
* @var \mysqli
*/
- protected $_conn;
+ protected $conn;
@stof
stof added a note

it cannot be removed from protected attributes because of BC (it is fine removing it from private though)

@kimhemsoe Collaborator

I asked @beberlei before i did it, but maybe he did know they were protected.

@stof
stof added a note

well, I think for this class it could be OK (I don't see any reason for extending the MysqliStatement class). The properties should even have been made private IMO (note that if we rename them, thus considering that nobody extend it, we could go further and make them private).
but renaming a protected property is a BC break (it impacts code extending the class), which is why I made my previous comment

@kimhemsoe Collaborator

I know.
They were made protected by someone from drupal. Think they ended up with never using any of it.
I love love to take some of them back to private and only leave _conn and _stmt. I dont see any reason why one would need the rest. I changed one of the protected were used the other day already there breaking the API (If you extend).

@kimhemsoe Collaborator

Should i change them all/some to private or leave it be ?

@deeky666 Collaborator

IMO we should leave the protected properties unchanged as we create more problems than benefits at this stage. I agree that those should be private but the codebase concerning private/protected, underscore/non-underscore is very inconsistent throughout the project anyways at this time. We should fix that in 3.0 as long as you don't have a certain reason why you need protected properties to be changed.
Still removing the underscores in private properties is fine for me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@kimhemsoe kimhemsoe closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
40 lib/Doctrine/DBAL/Driver/Mysqli/MysqliConnection.php
@@ -31,7 +31,7 @@ class MysqliConnection implements Connection, PingableConnection
/**
* @var \mysqli
*/
- private $_conn;
+ private $conn;
/**
* @param array $params
@@ -46,21 +46,21 @@ public function __construct(array $params, $username, $password, array $driverOp
$port = isset($params['port']) ? $params['port'] : ini_get('mysqli.default_port');
$socket = isset($params['unix_socket']) ? $params['unix_socket'] : ini_get('mysqli.default_socket');
- $this->_conn = mysqli_init();
+ $this->conn = mysqli_init();
$previousHandler = set_error_handler(function () {
});
- if ( ! $this->_conn->real_connect($params['host'], $username, $password, $params['dbname'], $port, $socket)) {
+ if ( ! $this->conn->real_connect($params['host'], $username, $password, $params['dbname'], $port, $socket)) {
set_error_handler($previousHandler);
- throw new MysqliException($this->_conn->connect_error, $this->_conn->connect_errno);
+ throw new MysqliException($this->conn->connect_error, $this->conn->connect_errno);
}
set_error_handler($previousHandler);
if (isset($params['charset'])) {
- $this->_conn->set_charset($params['charset']);
+ $this->conn->set_charset($params['charset']);
}
$this->setDriverOptions($driverOptions);
@@ -75,7 +75,7 @@ public function __construct(array $params, $username, $password, array $driverOp
*/
public function getWrappedResourceHandle()
{
- return $this->_conn;
+ return $this->conn;
}
/**
@@ -83,7 +83,7 @@ public function getWrappedResourceHandle()
*/
public function prepare($prepareString)
{
- return new MysqliStatement($this->_conn, $prepareString);
+ return new MysqliStatement($this->conn, $prepareString);
}
/**
@@ -103,7 +103,7 @@ public function query()
*/
public function quote($input, $type=\PDO::PARAM_STR)
{
- return "'". $this->_conn->escape_string($input) ."'";
+ return "'". $this->conn->escape_string($input) ."'";
}
/**
@@ -111,8 +111,8 @@ public function quote($input, $type=\PDO::PARAM_STR)
*/
public function exec($statement)
{
- $this->_conn->query($statement);
- return $this->_conn->affected_rows;
+ $this->conn->query($statement);
+ return $this->conn->affected_rows;
}
/**
@@ -120,7 +120,7 @@ public function exec($statement)
*/
public function lastInsertId($name = null)
{
- return $this->_conn->insert_id;
+ return $this->conn->insert_id;
}
/**
@@ -128,7 +128,7 @@ public function lastInsertId($name = null)
*/
public function beginTransaction()
{
- $this->_conn->query('START TRANSACTION');
+ $this->conn->query('START TRANSACTION');
return true;
}
@@ -137,7 +137,7 @@ public function beginTransaction()
*/
public function commit()
{
- return $this->_conn->commit();
+ return $this->conn->commit();
}
/**
@@ -145,7 +145,7 @@ public function commit()
*/
public function rollBack()
{
- return $this->_conn->rollback();
+ return $this->conn->rollback();
}
/**
@@ -153,7 +153,7 @@ public function rollBack()
*/
public function errorCode()
{
- return $this->_conn->errno;
+ return $this->conn->errno;
}
/**
@@ -161,7 +161,7 @@ public function errorCode()
*/
public function errorInfo()
{
- return $this->_conn->error;
+ return $this->conn->error;
}
/**
@@ -196,16 +196,16 @@ private function setDriverOptions(array $driverOptions = array())
);
}
- if (@mysqli_options($this->_conn, $option, $value)) {
+ if (@mysqli_options($this->conn, $option, $value)) {
continue;
}
$msg = sprintf($exceptionMsg, 'Failed to set', $option, $value);
- $msg .= sprintf(', error: %s (%d)', mysqli_error($this->_conn), mysqli_errno($this->_conn));
+ $msg .= sprintf(', error: %s (%d)', mysqli_error($this->conn), mysqli_errno($this->conn));
throw new MysqliException(
$msg,
- mysqli_errno($this->_conn)
+ mysqli_errno($this->conn)
);
}
}
@@ -217,6 +217,6 @@ private function setDriverOptions(array $driverOptions = array())
*/
public function ping()
{
- return $this->_conn->ping();
+ return $this->conn->ping();
}
}
View
96 lib/Doctrine/DBAL/Driver/Mysqli/MysqliStatement.php
@@ -41,27 +41,27 @@ class MysqliStatement implements \IteratorAggregate, Statement
/**
* @var \mysqli
*/
- protected $_conn;
+ protected $conn;
@stof
stof added a note

it cannot be removed from protected attributes because of BC (it is fine removing it from private though)

@kimhemsoe Collaborator

I asked @beberlei before i did it, but maybe he did know they were protected.

@stof
stof added a note

well, I think for this class it could be OK (I don't see any reason for extending the MysqliStatement class). The properties should even have been made private IMO (note that if we rename them, thus considering that nobody extend it, we could go further and make them private).
but renaming a protected property is a BC break (it impacts code extending the class), which is why I made my previous comment

@kimhemsoe Collaborator

I know.
They were made protected by someone from drupal. Think they ended up with never using any of it.
I love love to take some of them back to private and only leave _conn and _stmt. I dont see any reason why one would need the rest. I changed one of the protected were used the other day already there breaking the API (If you extend).

@kimhemsoe Collaborator

Should i change them all/some to private or leave it be ?

@deeky666 Collaborator

IMO we should leave the protected properties unchanged as we create more problems than benefits at this stage. I agree that those should be private but the codebase concerning private/protected, underscore/non-underscore is very inconsistent throughout the project anyways at this time. We should fix that in 3.0 as long as you don't have a certain reason why you need protected properties to be changed.
Still removing the underscores in private properties is fine for me :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
/**
* @var \mysqli_stmt
*/
- protected $_stmt;
+ protected $stmt;
/**
* @var null|boolean|array
*/
- protected $_columnNames;
+ protected $columnNames;
/**
* @var null|array
*/
- protected $_rowBindedValues;
+ protected $rowBindedValues;
/**
* @var array
*/
- protected $_bindedValues;
+ protected $bindedValues;
/**
* @var string
@@ -73,12 +73,12 @@ class MysqliStatement implements \IteratorAggregate, Statement
*
* @var array
*/
- protected $_values = array();
+ protected $values = array();
/**
* @var integer
*/
- protected $_defaultFetchMode = PDO::FETCH_BOTH;
+ protected $defaultFetchMode = PDO::FETCH_BOTH;
/**
* @param \mysqli $conn
@@ -88,16 +88,16 @@ class MysqliStatement implements \IteratorAggregate, Statement
*/
public function __construct(\mysqli $conn, $prepareString)
{
- $this->_conn = $conn;
- $this->_stmt = $conn->prepare($prepareString);
- if (false === $this->_stmt) {
- throw new MysqliException($this->_conn->error, $this->_conn->errno);
+ $this->conn = $conn;
+ $this->stmt = $conn->prepare($prepareString);
+ if (false === $this->stmt) {
+ throw new MysqliException($this->conn->error, $this->conn->errno);
}
- $paramCount = $this->_stmt->param_count;
+ $paramCount = $this->stmt->param_count;
if (0 < $paramCount) {
$this->types = str_repeat('s', $paramCount);
- $this->_bindedValues = array_fill(1 , $paramCount, null);
+ $this->bindedValues = array_fill(1 , $paramCount, null);
}
}
@@ -116,7 +116,7 @@ public function bindParam($column, &$variable, $type = null, $length = null)
}
}
- $this->_bindedValues[$column] =& $variable;
+ $this->bindedValues[$column] =& $variable;
$this->types[$column - 1] = $type;
return true;
@@ -137,8 +137,8 @@ public function bindValue($param, $value, $type = null)
}
}
- $this->_values[$param] = $value;
- $this->_bindedValues[$param] =& $this->_values[$param];
+ $this->values[$param] = $value;
+ $this->bindedValues[$param] =& $this->values[$param];
$this->types[$param - 1] = $type;
return true;
@@ -149,24 +149,24 @@ public function bindValue($param, $value, $type = null)
*/
public function execute($params = null)
{
- if (null !== $this->_bindedValues) {
+ if (null !== $this->bindedValues) {
if (null !== $params) {
if ( ! $this->_bindValues($params)) {
- throw new MysqliException($this->_stmt->error, $this->_stmt->errno);
+ throw new MysqliException($this->stmt->error, $this->stmt->errno);
}
} else {
- if (!call_user_func_array(array($this->_stmt, 'bind_param'), array($this->types) + $this->_bindedValues)) {
- throw new MysqliException($this->_stmt->error, $this->_stmt->errno);
+ if (!call_user_func_array(array($this->stmt, 'bind_param'), array($this->types) + $this->bindedValues)) {
+ throw new MysqliException($this->stmt->error, $this->stmt->errno);
}
}
}
- if ( ! $this->_stmt->execute()) {
- throw new MysqliException($this->_stmt->error, $this->_stmt->errno);
+ if ( ! $this->stmt->execute()) {
+ throw new MysqliException($this->stmt->error, $this->stmt->errno);
}
- if (null === $this->_columnNames) {
- $meta = $this->_stmt->result_metadata();
+ if (null === $this->columnNames) {
+ $meta = $this->stmt->result_metadata();
if (false !== $meta) {
$columnNames = array();
foreach ($meta->fetch_fields() as $col) {
@@ -174,25 +174,25 @@ public function execute($params = null)
}
$meta->free();
- $this->_columnNames = $columnNames;
- $this->_rowBindedValues = array_fill(0, count($columnNames), NULL);
+ $this->columnNames = $columnNames;
+ $this->rowBindedValues = array_fill(0, count($columnNames), NULL);
$refs = array();
- foreach ($this->_rowBindedValues as $key => &$value) {
+ foreach ($this->rowBindedValues as $key => &$value) {
$refs[$key] =& $value;
}
- if (!call_user_func_array(array($this->_stmt, 'bind_result'), $refs)) {
- throw new MysqliException($this->_stmt->error, $this->_stmt->errno);
+ if (!call_user_func_array(array($this->stmt, 'bind_result'), $refs)) {
+ throw new MysqliException($this->stmt->error, $this->stmt->errno);
}
} else {
- $this->_columnNames = false;
+ $this->columnNames = false;
}
}
// We have a result.
- if (false !== $this->_columnNames) {
- $this->_stmt->store_result();
+ if (false !== $this->columnNames) {
+ $this->stmt->store_result();
}
return true;
@@ -215,7 +215,7 @@ private function _bindValues($values)
$params[] =& $v;
}
- return call_user_func_array(array($this->_stmt, 'bind_param'), $params);
+ return call_user_func_array(array($this->stmt, 'bind_param'), $params);
}
/**
@@ -223,11 +223,11 @@ private function _bindValues($values)
*/
private function _fetch()
{
- $ret = $this->_stmt->fetch();
+ $ret = $this->stmt->fetch();
if (true === $ret) {
$values = array();
- foreach ($this->_rowBindedValues as $v) {
+ foreach ($this->rowBindedValues as $v) {
$values[] = $v;
}
return $values;
@@ -247,20 +247,20 @@ public function fetch($fetchMode = null)
}
if (false === $values) {
- throw new MysqliException($this->_stmt->error, $this->_stmt->errno);
+ throw new MysqliException($this->stmt->error, $this->stmt->errno);
}
- $fetchMode = $fetchMode ?: $this->_defaultFetchMode;
+ $fetchMode = $fetchMode ?: $this->defaultFetchMode;
switch ($fetchMode) {
case PDO::FETCH_NUM:
return $values;
case PDO::FETCH_ASSOC:
- return array_combine($this->_columnNames, $values);
+ return array_combine($this->columnNames, $values);
case PDO::FETCH_BOTH:
- $ret = array_combine($this->_columnNames, $values);
+ $ret = array_combine($this->columnNames, $values);
$ret += $values;
return $ret;
@@ -274,7 +274,7 @@ public function fetch($fetchMode = null)
*/
public function fetchAll($fetchMode = null)
{
- $fetchMode = $fetchMode ?: $this->_defaultFetchMode;
+ $fetchMode = $fetchMode ?: $this->defaultFetchMode;
$rows = array();
if (PDO::FETCH_COLUMN == $fetchMode) {
@@ -308,7 +308,7 @@ public function fetchColumn($columnIndex = 0)
*/
public function errorCode()
{
- return $this->_stmt->errno;
+ return $this->stmt->errno;
}
/**
@@ -316,7 +316,7 @@ public function errorCode()
*/
public function errorInfo()
{
- return $this->_stmt->error;
+ return $this->stmt->error;
}
/**
@@ -324,7 +324,7 @@ public function errorInfo()
*/
public function closeCursor()
{
- $this->_stmt->free_result();
+ $this->stmt->free_result();
return true;
}
@@ -334,10 +334,10 @@ public function closeCursor()
*/
public function rowCount()
{
- if (false === $this->_columnNames) {
- return $this->_stmt->affected_rows;
+ if (false === $this->columnNames) {
+ return $this->stmt->affected_rows;
}
- return $this->_stmt->num_rows;
+ return $this->stmt->num_rows;
}
/**
@@ -345,7 +345,7 @@ public function rowCount()
*/
public function columnCount()
{
- return $this->_stmt->field_count;
+ return $this->stmt->field_count;
}
/**
@@ -353,7 +353,7 @@ public function columnCount()
*/
public function setFetchMode($fetchMode, $arg2 = null, $arg3 = null)
{
- $this->_defaultFetchMode = $fetchMode;
+ $this->defaultFetchMode = $fetchMode;
return true;
}
Something went wrong with that request. Please try again.