Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor PredicateBuilder from singleton to instance #17886

Merged
merged 1 commit into from
Dec 2, 2014

Conversation

mrgilman
Copy link

@mrgilman mrgilman commented Dec 2, 2014

No description provided.

@sgrif sgrif self-assigned this Dec 2, 2014
@rafaelfranca
Copy link
Member

Thanks! What is the reason for this change?

@rafaelfranca
Copy link
Member

In fact this is reverting a change @tenderlove made some time ago to avoid instantiating objects 83633b8

@sgrif
Copy link
Contributor

sgrif commented Dec 2, 2014

I had asked her to work on this, it's part of the larger refactoring to remove type casting from Arel. One of the things we need to do in order to make that easier is remove the duplication inside of the ArrayHandler with regards to ranges and single value equality. The easiest path to do that is to pass the predicate builder into the handler, but for that to work we need a single method we can call with reasonable parameters.

@sgrif
Copy link
Contributor

sgrif commented Dec 2, 2014

We can probably reduce this down to a single allocation per AR class later to keep the work of 83633b8

@rafaelfranca
Copy link
Member

👍, just wondering what was the reason. The code seems good to me.

sgrif added a commit that referenced this pull request Dec 2, 2014
Refactor `PredicateBuilder` from singleton to instance
@sgrif sgrif merged commit 3317d69 into rails:master Dec 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants