unix1 / spoof Goto Github PK

Rather than having a project-specific code style standard, it should follow http://www.php-fig.org/psr/psr-2/ instead.

Should there be a NotFoundException type that is implemented by both data\RecordNotFoundException and api\ResourceNotFoundException? This would make it easier to capture a general "not found" condition for API response.

Similar to DatabaseTestCase class there should be a base test class containing common base test functionality. First obvious candidate is getProtectedProperty methods spread around individual test classes.

Currently Connection abstract class checks whether a connection is active with
is_null($this->connection)
TODO in comments asks whether there is a better way of doing this.

The spoof\api\* classes currently have no unit test coverage.

Per code climate $o variable name is too short, replace it.

To repro:

• run unit tests
  cd lib360
phpunit
• look at additional files created
  git status

Expected: no additional files created

Actual: lib360/tests/db/test.db remains

test.db file must be deleted after the test run is completed.

Model class wraps Record, which makes the record array not easily accessible. There should probably be an easy way to transform an instance of a model into an array for output and other external uses.

Per code climate report, short variable name $q, mostly used in SQL class is too short. Replace it with $query.

The getFeatureLevel methods were added but not fully developed or used at all. Remove it for now.

The following methods

Table::select
Table::insert
Table::update
Table::delete

expect values to be instances of IValue. However, the following methods:

Table::selectRecord
Table::insertRecord
Table::updateRecord

expect values to be plain strings. Also, when creating conditions and field maps they are hardcoded to strings.

How should this be made consistent? One option is to add automatic type detection to Value class, so it works when data is set to a Record instance too.

Support SQL LIKE and NOT LIKE operators.

Currently Connection abstract class constructor tries to read the configuration object DSN and parse the driver name and load the driver object.

Issues are:

• this process might not be the same for all connection implementations.
• it would be nicer to explicitly allow each implementation an option to perform its own parsing instead of trying to override the constructor

Currently TODO in Connection::__construct states/asks to implement additional logging when a driver specified in db\Config object is not found. In this case object\Factory throws an object\NotFoundException.

To implement:

• this should not log anything: remove comment about logging
• should the Connection object catch and rethrow exception, or let it propagate?

Catching and rethrowing would allow the exception to contain more details about the DSN containing the error.

The executor\PDO class has some code duplication between insert, update, delete, select, and query methods. Create smaller functions to get rid of the code duplication.

Model classes should build on existing Table and Record classes to make operating on data easier. User should only need to define a model class, tie it to a table containing a key and a list of fields. Model should take care of all CRUD functions, including operating on lists of data via RecordList.