Hi Lukas
Thanks for adding this - it's definitely something we needed in ObjectiveCMIS!
Regarding the implementation, I have a few comments:
1 - My main concern is whether using '?' as token in "SELECT ?, ? FROM ? WHERE
? > ? AND IN_FOLDER(?) OR ? IN (?)" is too fragile?
We have a similar token replacement scenario in our mobile app whilst parsing
user activity data, but in that case we use a numbered indexing scheme, e.g.
"{1} renamed wiki page from {2} to {0}". This has several advantages from my
point of view: it means the string can be reformatted without manual reordering
of parameter passing and also makes the parameters within the string easier to
identify (instead of counting '?'s). Tokens are easily found using
rangeOfString:
NSRange indexRange = [[inStr string] rangeOfString:[NSString
stringWithFormat:@"{%@}", @(index)]];
It would also be possible to write the query generator to allow parameter
re-use within the string without having to escape the same parameter again.
2 - I notice convert: is allocating an NSDateFormatter each time it's called.
Apple recommend that NSDateFormatters are cached and reused, see [1]
[1]
https://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/DataFormatting/Articles/dfDateFormatting10_4.html#//apple_ref/doc/uid/TP40002369-SW10
3 - It's a shame that new code in ObjectiveCMIS isn't being written using
modern syntax, for example:
[self.parametersDictionary setObject:[CMISQueryStatement escapeString:type
withSurroundingQuotes:NO] forKey:[NSNumber numberWithInteger:parameterIndex]];
becomes
self.parametersDictionary[@(parameterIndex)] = [CMISQueryStatement
escapeString:type withSurroundingQuotes:NO];
4 - Finally, a minor point; I think some line breaks got lost in the formatting
for the setStringContainsAtIndex:string: documentation:
* Summary (input --> first level escaping --> second level escaping and
* output): * --> * --> * ? --> ? --> ? - --> - --> - \ --> \\ --> \\\\ (for
* any other character following other than * ? -) \* --> \* --> \\* \? -->
* \? --> \\? \- --> \- --> \\- ' --> \' --> \\\' " --> \" --> \\\"
Thanks,
Mike
On 17 Jul 2014, at 16:59, Gross, Lukas
<[email protected]<mailto:[email protected]>> wrote:
Hi,
I added CMISQueryStatement class to enable users to escape their queries using
kind of a prepared statement approach.
Please let me know if you have any objections…
I plan to trigger the .4 release soon, so please have a look :)
Best regards,
Lukas
