Skip to content

Commit

Permalink
perf(QueryBuilder): Remove variadic parameter support
Browse files Browse the repository at this point in the history 
Variadic parameter support added 100-200ms a call.
It's not worth it.

BREAKING CHANGE: Remove variadic parameter support
  • Loading branch information
@elpete
elpete committed Oct 2, 2019
1 parent bae3435 commit 8690fcf
Show file tree
Hide file tree
Showing 7 changed files with 27 additions and 126 deletions.
110 changes: 19 additions & 91 deletions models/Query/QueryBuilder.cfc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,19 +235,10 @@ component displayname="QueryBuilder" accessors="true" {
* @return qb.models.Query.QueryBuilder
*/
public QueryBuilder function select( any columns = "*" ) {
// This block is necessary for ACF 10.
// It can't be extracted to a function because
// the arguments struct doesn't get passed correctly.
var args = {};
var count = structCount( arguments );
for ( var arg in arguments ) {
args[ count ] = arguments[ arg ];
count--;
}

variables.columns = normalizeToArray( argumentCollection = args ).map( function( column ) {
return applyColumnFormatter( column );
} );
variables.columns = normalizeToArray( arguments.columns )
.map( function( column ) {
return applyColumnFormatter( column );
} );
if ( variables.columns.isEmpty() ) {
variables.columns = [ "*" ];
}
Expand Down Expand Up @@ -291,21 +282,13 @@ component displayname="QueryBuilder" accessors="true" {
* @return qb.models.Query.QueryBuilder
*/
public QueryBuilder function addSelect( required any columns ) {
// This block is necessary for ACF 10.
// It can't be extracted to a function because
// the arguments struct doesn't get passed correctly.
var args = {};
var count = structCount( arguments );
for ( var arg in arguments ) {
args[ count ] = arguments[ arg ];
count--;
}

if ( variables.columns.isEmpty() ||
( variables.columns.len() == 1 && isSimpleValue( variables.columns[ 1 ] ) && variables.columns[ 1 ] == "*" ) ) {
variables.columns = [];
}
var newColumns = normalizeToArray( argumentCollection = args ).map( applyColumnFormatter );
var newColumns = normalizeToArray( arguments.columns ).map( function( column ) {
return applyColumnFormatter( column );
} );
arrayAppend( variables.columns, newColumns, true );
return this;
}
Expand Down Expand Up @@ -1569,21 +1552,11 @@ component displayname="QueryBuilder" accessors="true" {
*
* @return qb.models.Query.QueryBuilder
*/
public QueryBuilder function groupBy() {
// This block is necessary for ACF 10.
// It can't be extracted to a function because
// the arguments struct doesn't get passed correctly.
var args = {};
var count = 1;
for ( var arg in arguments ) {
args[ count ] = arguments[ arg ];
count++;
}

var groupBys = normalizeToArray( argumentCollection = args );
groupBys.each( function( groupBy ) {
public QueryBuilder function groupBy( required groups ) {
var groupBys = normalizeToArray( arguments.groups );
for ( var groupBy in groupBys ) {
variables.groups.append( applyColumnFormatter( groupBy ) );
} );
}
return this;
}

Expand Down Expand Up @@ -2624,63 +2597,18 @@ component displayname="QueryBuilder" accessors="true" {
*
* @return array
*/
private array function normalizeToArray() {
if ( isVariadicFunction( args = arguments ) ) {
return normalizeVariadicArgumentsToArray( args = arguments );
}

var arg = arguments[ 1 ];
if ( getUtils().isExpression( arg ) ) {
return [ arg ];
private array function normalizeToArray( required listOrArray ) {
if ( isArray( arguments.listOrArray ) ) {
return arguments.listOrArray;
}

if ( ! isArray( arg ) ) {
return normalizeListArgumentsToArray( arg );
}

return arg;
}

/**
* Returns true if the arguments passed constitute a variadic function.
*
* @args The arguments of another function.
*
* @return boolean
*/
private boolean function isVariadicFunction( required struct args ) {
return structCount( args ) > 1;
}

/**
* Converts a variadic list of arguments to an array.
*
* @args The arguments of another function.
*
* @return array
*/
private array function normalizeVariadicArgumentsToArray( required struct args ) {
var normalizedArgs = [];
for ( var arg in arguments.args ) {
arrayAppend( normalizedArgs, arguments.args[ arg ] );
if ( getUtils().isExpression( arguments.listOrArray ) ) {
return [ arguments.listOrArray ];
}
return normalizedArgs;
}

/**
* Converts a list of arguments to an array.
*
* @list A list containing multiple arguments.
*
* @return array
*/
private array function normalizeListArgumentsToArray( required string list ) {
var listAsArray = listToArray( arguments.list );
var items = [];
for ( var item in listAsArray ) {
arrayAppend( items, trim( item ) );
}
return items;
var cfArray = [];
cfArray.append( arguments.listOrArray.split( ",\s*" ), true );
return cfArray;
}

/**
Expand Down
17 changes: 8 additions & 9 deletions tests/resources/AbstractQueryBuilderSpec.cfc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@ component extends="testbox.system.BaseSpec" {
}, selectSpecificColumn() );
} );

it( "can select multiple columns using variadic parameters", function() {
testCase( function( builder ) {
builder.select( "id", "name" ).from( "users" );
}, selectMultipleVariadic() );
} );

it( "can select multiple columns using an array", function() {
testCase( function( builder ) {
builder.select( [ "name", builder.raw( "COUNT(*)" ) ] ).from( "users" );
Expand All @@ -46,7 +40,7 @@ component extends="testbox.system.BaseSpec" {

it( "can select distinct records", function() {
testCase( function( builder ) {
builder.distinct().select( "foo", "bar" ).from( "users" );
builder.distinct().select( [ "foo", "bar" ] ).from( "users" );
}, selectDistinct() );
} );

Expand Down Expand Up @@ -154,7 +148,10 @@ component extends="testbox.system.BaseSpec" {

it( "can specify the table using fromSub as QueryBuilder", function() {
testCase( function( builder ) {
var derivedTable = getBuilder().select("id", "name").from("users").where( "age", ">=", "21" );
var derivedTable = getBuilder()
.select( [ "id", "name" ] )
.from( "users" )
.where( "age", ">=", "21" );

builder.fromSub( "u", derivedTable );
}, fromDerivedTable() );
Expand All @@ -163,7 +160,9 @@ component extends="testbox.system.BaseSpec" {
it( "can specify the table using fromSub as a closure", function() {
testCase( function( builder ) {
builder.fromSub( "u", function (q){
q.select("id", "name").from("users").where( "age", ">=", "21" );
q.select( [ "id", "name" ] )
.from( "users" )
.where( "age", ">=", "21" );
} );
}, fromDerivedTable() );
} );
Expand Down
10 changes: 0 additions & 10 deletions tests/specs/Query/Abstract/BuilderSelectSpec.cfc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,6 @@ component extends="testbox.system.BaseSpec" {
query.select( [ "::some_column::", "::another_column::" ] );
expect( query.getColumns() ).toBe( [ "::some_column::", "::another_column::" ] );
} );

it( "using variadic parameters", function() {
query.select( "::some_column::", "::another_column::" );
expect( query.getColumns() ).toBe( [ "::some_column::", "::another_column::" ] );
} );
} );
} );

Expand All @@ -55,11 +50,6 @@ component extends="testbox.system.BaseSpec" {
query.addSelect( [ "::another_column::", "::yet_another_column::" ] );
expect( query.getColumns() ).toBe( [ "::some_column::", "::another_column::", "::yet_another_column::" ] );
} );

it( "using variadic parameters", function() {
query.addSelect( "::another_column::", "::yet_another_column::" );
expect( query.getColumns() ).toBe( [ "::some_column::", "::another_column::", "::yet_another_column::" ] );
} );
} );
} );

Expand Down
4 changes: 0 additions & 4 deletions tests/specs/Query/MSSQLQueryBuilderSpec.cfc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ component extends="tests.resources.AbstractQueryBuilderSpec" {
return "SELECT [name] FROM [users]";
}

function selectMultipleVariadic() {
return "SELECT [id], [name] FROM [users]";
}

function selectMultipleArray() {
return "SELECT [name], COUNT(*) FROM [users]";
}
Expand Down
4 changes: 0 additions & 4 deletions tests/specs/Query/MySQLQueryBuilderSpec.cfc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ component extends="tests.resources.AbstractQueryBuilderSpec" {
return "SELECT `name` FROM `users`";
}

function selectMultipleVariadic() {
return "SELECT `id`, `name` FROM `users`";
}

function selectMultipleArray() {
return "SELECT `name`, COUNT(*) FROM `users`";
}
Expand Down
4 changes: 0 additions & 4 deletions tests/specs/Query/OracleQueryBuilderSpec.cfc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ component extends="tests.resources.AbstractQueryBuilderSpec" {
return "SELECT ""NAME"" FROM ""USERS""";
}

function selectMultipleVariadic() {
return "SELECT ""ID"", ""NAME"" FROM ""USERS""";
}

function selectMultipleArray() {
return "SELECT ""NAME"", COUNT(*) FROM ""USERS""";
}
Expand Down
4 changes: 0 additions & 4 deletions tests/specs/Query/PostgresQueryBuilderSpec.cfc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ component extends="tests.resources.AbstractQueryBuilderSpec" {
return "SELECT ""name"" FROM ""users""";
}

function selectMultipleVariadic() {
return "SELECT ""id"", ""name"" FROM ""users""";
}

function selectMultipleArray() {
return "SELECT ""name"", COUNT(*) FROM ""users""";
}
Expand Down

0 comments on commit 8690fcf

Please sign in to comment.