This code looks good. Just a few possible improvements, in the spirit of
code-review:
On Jan 29, 2010, at 4:00 AM, Keith Blount wrote:
> NSMutableCharacterSet *XMLCharacterSet =
> [[NSMutableCharacterSet alloc] init];
Variable names shouldn't start with an uppercase letter — the convention is to
lowercase the entire acronym that starts a name, so "xmlCharacterSet".
> range = [cleanedString
> rangeOfCharacterFromSet:invalidXMLCharacterSet];
FYI: If you ever find this method is a performance bottleneck, you can make it
run faster on long strings by using the variant of this method that takes a
range, and passing in the range from the last deletion point to the end of the
string. That will avoid scanning the already-cleaned prefix again.
> return (NSString *)[cleanedString autorelease];
The cast isn't necessary; an NSMutableString is an NSString.
I find it more maintainable to do the autorelease at the same time I create the
object, i.e. change the declaration to
NSMutableString *cleanedString = [[self mutableCopy] autorelease];
and then just "return cleanedString" at the end. That way if you (or someone
else) every modifies the code later to add an earlier 'return' statement, they
won't accidentally create a memory leak if they forget to autorelease it.
—Jens_______________________________________________
Cocoa-dev mailing list ([email protected])
Please do not post admin requests or moderator comments to the list.
Contact the moderators at cocoa-dev-admins(at)lists.apple.com
Help/Unsubscribe/Update your Subscription:
http://lists.apple.com/mailman/options/cocoa-dev/archive%40mail-archive.com
This email sent to [email protected]
