-
Notifications
You must be signed in to change notification settings - Fork 520
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
Enabling SOQL functions ( closes #163 ) #258
base: master
Are you sure you want to change the base?
Enabling SOQL functions ( closes #163 ) #258
Conversation
…nd re-adding after matched token * fflib_QueryFactory.cls: Added matching Pattern for all SOQL Functions (COUNT_DISTINCT, toLabel, MIN, MAX, etc.) * fflib_QueryFactoryTest.cls: Adding test methods to verify correct behavior
Merging apex-common back into personal fork
return soqlFunctionPattern; | ||
} set; } | ||
|
||
private String getFieldPath( String fieldName ){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By adding this logic inside this method the complexity suddenly becomes very high. I would advise to split this logic into a number of private methods to keep a level of abstraction here in his method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wimvelzeboer I do agree with you on the increased complexity and length of the method. I've thought of this before, but in my perspective this will only become more difficult to read, since beforeField
and afterField
are used throughout the function. The methods should then return a list/map or an innerclass to support. That's why I've put emphasize on the comments.
If you have any suggestion how to split and make sure all data is properly fed back to be used, please let me know!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would maybe split up this method into three parts:
private String getFieldPathForMethod(String fieldName)
private String getFieldPathForRelatedField(String fieldName)
private String getFieldPathForField(String fieldName)
private String getFieldPath( String fieldName )
{
if (fieldName.contains( '(' )) return getFieldPathForMethod(fieldName);
if (fieldName.contains( '.' ) ) return getFieldPathForRelatedField(fieldName);
return getFieldPathForField(fieldName);
}
private getFieldPathForMethod(String methodCall)
{
Matcher match = this.soqlFunctionPattern.matcher( methodCall );
if (!match.matches())
{
throw new InvalidFieldException( 'The method '+ methodCall + 'does specify a ( function opening, but doesn\'t match the pattern' );
}
String beforeField = match.group( 1 );
String fieldName = match.group( 2 );
String afterField = match.group( 3 );
// When no fieldName is specified (e.g. COUNT() ), return directly, since nothing to check
if (String.isBlank( fieldName ))
{
return beforeField + afterField;
}
return beforeField + getFieldPath(fieldName) + afterField;
}
Enrichment to the fflib Query Factory to allow soql methods (like COUNT(), toLabel, and such) to be specified. Note, they can be added via
selectFields( String(s) )
methods, as is illustrated in the provided test methods.This change is