Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Zend\Db\Predicates - allow use different types of arguments in any position #5320

Closed
wants to merge 3 commits into from
Closed

Zend\Db\Predicates - allow use different types of arguments in any position #5320

wants to merge 3 commits into from

Conversation

turrsis
Copy link
Contributor

@turrsis turrsis commented Oct 22, 2013

for more readable methods calls
implements Case predicate to demonstrate the need for these changes

$res .= $this->resolveSqlString($query) . ';';
}
return $res;
} elseif($sql instanceof SqlInterface) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after elseif and before open parenthesis

@tca3
Copy link
Contributor

tca3 commented Oct 25, 2013

I stopped looking up the code because it's breaking BC too much, you can't alter method declarations like this (method names, arguments) or even strip interfaces out of nowhere, they're here for a reason :)

@turrsis
Copy link
Contributor Author

turrsis commented Oct 28, 2013

simplification and cs fixes

@ralphschindler
Copy link
Member

I'm not quite sure what the whole of the use cases are, but I see a few things wrong with the code in here. Namely, it looks like Zend\Db\Adapter has an awareness of Zend\Db\Sql components. Zend\Db\Sql components should have a dependency on Zend\Db\Adapter, but not visa-versa.

Can you better explain the use cases with some examples and perhaps we can find a better way to implement them?

@turrsis
Copy link
Contributor Author

turrsis commented Nov 19, 2013

This PR is child of the #5306.
It will be split about some days

@turrsis
Copy link
Contributor Author

turrsis commented Nov 24, 2013

@ralphschindler simplified

@weierophinney
Copy link
Member

@ralphschindler Can you update with the appropriate milestone please, and/or close?

@ralphschindler ralphschindler added this to the 3.0.0 milestone Mar 4, 2014
@ralphschindler ralphschindler self-assigned this Mar 4, 2014
self::TYPE_VALUE,
);

protected function normalizeArgument($argument, $defaultType = self::TYPE_VALUE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a docblock, please.

@weierophinney
Copy link
Member

@turrsis From what I can see, this is mainly a refactoring in order to reduce duplication; I don't see any new functionality. Is that correct? If so, I can schedule for 2.4.0.

@turrsis
Copy link
Contributor Author

turrsis commented May 13, 2014

Yes, this is refactoring and reduce duplication.
But this is point of extention too. Because we need Case predicate, which can use many parameters and it constructor should be readable.

@Ocramius Ocramius assigned Ocramius and unassigned weierophinney Nov 12, 2014
Ocramius added a commit that referenced this pull request Nov 12, 2014
Ocramius added a commit that referenced this pull request Nov 12, 2014
Ocramius added a commit that referenced this pull request Nov 12, 2014
Ocramius added a commit that referenced this pull request Nov 12, 2014
Ocramius added a commit that referenced this pull request Nov 12, 2014
Ocramius added a commit that referenced this pull request Nov 12, 2014
Ocramius added a commit that referenced this pull request Nov 12, 2014
Ocramius added a commit that referenced this pull request Nov 12, 2014
@Ocramius
Copy link
Member

Manually merged in 35ec442

Thanks!

@Ocramius Ocramius closed this Nov 12, 2014
@turrsis turrsis deleted the feature/db-sql-functions branch November 18, 2014 18:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants