Re: Cache Class review (low priority)

Jens Alfke Wed, 30 Apr 2008 08:08:23 -0700

I can't say whether this code will work or not, but it has quite a lot of problems with its design.

* As Jean-Daniel pointed out, most of what you're doing here is re- implementing NSMutableDictionary, only much less efficiently (by using linear search instead of hashing.)

* Doing the periodic 'touch' operation by running a separate thread is expensive and then requires you to put synchronization code all over the place. Just use an NSTimer instead.

* It looks as though your class uses static variables instead of instance variables. Even though this is a singleton object, that's a really bad design. If you ever wanted to re-use some of this code for a non-singleton purpose, you'd have to rewrite it. An object's state should be stored in its ivars.

* Similarly, don't put instance initialization code in +alloc. It goes in -init.

* Naming static variables in all caps is confusing. All-caps is normally only used for preprocessor macros. Static/global variables are more commonly either capitalized or use a single-letter prefix like "g" or "s".


* This is not going to work correctly for thread synchronization:

                // wait for the array to be unlocked
                do {
                } while (arrayLocked == 1);

The effects of threads reading and writing a shared memory location are pretty subtle and machine-dependent. To get this to work reliably you'd either have to insert memory barriers, or better, replace this with an @synchronized or an NSLock, or best, get rid of the threading completely by using NSTimer. * Comments that just describe literally what a single line is doing are not useful; they just make the code take longer to read. It's already evident that this line gets the count; there's no need to repeat yourself:

                // get the count of the array
                items = [THE_ARRAY count];


—Jens

smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________

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]

Re: Cache Class review (low priority)

Reply via email to