Db\Sql - cleaning code duplicates #5701

turrsis · 2014-01-10T01:28:07Z

moved to functions :

resolving columns (AbstractSql::resolveColumnValue)
resolving $this->table (AbstractSql::resolveTable)
resolve $parameterContainer (AbstractSql::resolveParameterContainer)

Insert reorganized because :

for using common method for resolving columns
use of $columns and $values is redundant, $columns only is enough

AbstractSql::processExpression and AbstractSql::processSubSelect : do more readable

Added new commits (step 6 - step 9):

delete dependency between $driver and $parameterContainer for process... methods.
move prepareStatement() and getSqlString() to AbstractSql because it duplicates in many places
AbstractSql::processExpression() should return string and paremeters push to ParameterContainer directly - this remove excess ParameterContainer::merge() calls and simplificate code
some code optimization

ralphschindler · 2014-01-13T16:41:44Z

Big commit, have to check if there are any BC issues. I realize you like creating abstractions for non-algorithmically interesting code blocks, so I'll defer to the cr-team on that stuff. Personally, I like shallower stack traces when I am having to debug through stuff, especially something as simple as the workflow you created for on resolving parameter containers. But that is my personal preference.

As for some of the rest, it looks ok- considering the size, it might take some time to get to.

weierophinney · 2014-03-03T18:15:25Z

@ralphschindler Can you set a milestone on this, please?

weierophinney · 2014-05-12T21:54:45Z

library/Zend/Db/Sql/AbstractSql.php

@@ -199,4 +198,65 @@ protected function processSubSelect(Select $subselect, PlatformInterface $platfo
        }
        return $sql;
    }
+
+    protected function resolveColumnValue($column, PlatformInterface $platform, DriverInterface $driver = null, $namedParameterPrefix = null, ParameterContainer $parameterContainer = null)


doc block, please.

On all new methods...

weierophinney · 2014-05-12T22:05:53Z

This looks good, and like it may offer some performance improvements due to reduced algorithmic complexity and duplication of records.

weierophinney · 2014-05-12T22:06:15Z

@turrsis Ping me when the docblocks are in place, and I'll merge.

turrsis · 2014-05-13T08:30:30Z

@weierophinney the docblocks added

turrsis · 2014-05-13T10:45:06Z

@weierophinney please, see this #6262 (this is reduced algorithmic too). Or add new commit to current PR?

turrsis · 2014-05-24T11:25:10Z

@weierophinney Can you merge this (docblocks are in place)?

turrsis · 2014-05-27T12:53:43Z

Added new commits (step 6 - step 9), and change PR description

turrsis · 2014-05-29T07:59:08Z

Added new commits (step 10 - step 11)

remove $driver from IF which switch between getSqlString() and prepareStatement() algorithm

…ush to ParameterContainer directly

…from Platform decorators, unification name of the $subject variable

- move $this->processInfo['paramPrefix'] to AbstractSql class - AbstractSql::resolveColumnValue() - move $namedParameterPrefix parameter to last position

…use this is Select::processJoins() dublicate

…ed assignments

…velop Close #5701

Ocramius · 2014-11-13T12:45:35Z

This was rebased and manually merged into develop at 6c20013

@turrsis thanks!

Maks3w assigned ralphschindler Feb 16, 2014

Maks3w added the Db label Feb 16, 2014

ralphschindler added this to the 3.0.0 milestone Mar 4, 2014

weierophinney reviewed May 12, 2014
View reviewed changes

weierophinney modified the milestones: 2.4.0, 3.0.0 May 12, 2014

weierophinney assigned weierophinney and unassigned ralphschindler May 12, 2014

weierophinney added the awaiting author updates label May 12, 2014

turrsis mentioned this pull request May 27, 2014

Db\Sql delete parameters dependency #6262

Closed

EvanDotPro force-pushed the develop branch from 8f4d208 to a2c77c3 Compare September 28, 2014 00:34

turrsis added 10 commits November 12, 2014 15:50

step 1 : cleaning Insert

566bda8

step 2 : cleaning functions

5d8fab9

step 3 : resolve ParameterContainer

81b44a4

step 4 : resolve Table

3d99697

step 5 : fix columns with table identifier

dbfdc04

step 6 : delete parameters dependency

2f3077e

remove $driver from IF which switch between getSqlString() and prepareStatement() algorithm

step 7 : move prepareStatement() and getSqlString() to AbstractSql

8bd89d5

step 8 : AbstractSql::processExpression() return string, paremeters p…

8a02f60

…ush to ParameterContainer directly

step 9 : code optimization

478c301

step 10 : remove duplicates from AbstractSql::processSubSelect() and …

a69aeec

…from Platform decorators, unification name of the $subject variable

turrsis added 7 commits November 12, 2014 15:50

step 11 : remove duplicates from AbstractPlatform

b89c82b

step 12 : move localize variables to AbstractSql::localizeVariables()

0ef8cb3

step 13 : optimize functions parameters

d0fa0df

- move $this->processInfo['paramPrefix'] to AbstractSql class - AbstractSql::resolveColumnValue() - move $namedParameterPrefix parameter to last position

step 14 : delete Platform\Oracle\SelectDecorator::processJoins() beca…

f107d8e

…use this is Select::processJoins() dublicate

step 15 : add functional tests

522d429

fix doc block

e959611

fix after rebase and merge RP#5142

900bd25

Ocramius assigned Ocramius and unassigned weierophinney Nov 13, 2014

Ocramius added a commit that referenced this pull request Nov 13, 2014

#5701 - removing duplicate comments, minor CS cleanups, removing unus…

33a119e

…ed assignments

Ocramius added a commit that referenced this pull request Nov 13, 2014

#5701 - correcting type-hint for nullable value

adefbda

Ocramius added a commit that referenced this pull request Nov 13, 2014

#5701 - minor CS fixes (spacing/docblocks)

04c2aa0

Ocramius added a commit that referenced this pull request Nov 13, 2014

#5701 - adding missing docblocks, cleaning up docblock spec

126c526

Ocramius added a commit that referenced this pull request Nov 13, 2014

#5701 - adding missing docblocks, cleaning up docblock spec

90f3b40

Ocramius added a commit that referenced this pull request Nov 13, 2014

#5701 - making fluent interface explicit

c35bc0a

Ocramius added a commit that referenced this pull request Nov 13, 2014

#5701 - additional docblocks de-duplication and fixes

0249ab6

Ocramius added a commit that referenced this pull request Nov 13, 2014

Merge branch 'hotfix/#5701-zend-db-code-duplication-cleanups' into de…

6c20013

…velop Close #5701

Ocramius closed this Nov 13, 2014

Ocramius added enhancement and removed awaiting author updates labels Nov 13, 2014

turrsis deleted the hotfix/db-sql-cleaning branch November 18, 2014 18:05

turrsis mentioned this pull request Dec 16, 2014

[Db\Sql\Platform\Mysql\Ddl] override getSqlString() is wrong #7038

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Db\Sql - cleaning code duplicates #5701

Db\Sql - cleaning code duplicates #5701

turrsis commented Jan 10, 2014

ralphschindler commented Jan 13, 2014

weierophinney commented Mar 3, 2014

weierophinney May 12, 2014

weierophinney May 12, 2014

weierophinney commented May 12, 2014

weierophinney commented May 12, 2014

turrsis commented May 13, 2014

turrsis commented May 13, 2014

turrsis commented May 24, 2014

turrsis commented May 27, 2014

turrsis commented May 29, 2014

Ocramius commented Nov 13, 2014

Db\Sql - cleaning code duplicates #5701

Db\Sql - cleaning code duplicates #5701

Conversation

turrsis commented Jan 10, 2014

ralphschindler commented Jan 13, 2014

weierophinney commented Mar 3, 2014

weierophinney May 12, 2014

Choose a reason for hiding this comment

weierophinney May 12, 2014

Choose a reason for hiding this comment

weierophinney commented May 12, 2014

weierophinney commented May 12, 2014

turrsis commented May 13, 2014

turrsis commented May 13, 2014

turrsis commented May 24, 2014

turrsis commented May 27, 2014

turrsis commented May 29, 2014

Ocramius commented Nov 13, 2014