• Open Menu Close Menu
  • Apple
  • Shopping Bag
  • Apple
  • Mac
  • iPad
  • iPhone
  • Watch
  • TV
  • Music
  • Support
  • Search apple.com
  • Shopping Bag

Lists

Open Menu Close Menu
  • Terms and Conditions
  • Lists hosted on this site
  • Email the Postmaster
  • Tips for posting to public mailing lists
Re: Cache Class review (low priority)
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Cache Class review (low priority)


  • Subject: Re: Cache Class review (low priority)
  • From: Jens Alfke <email@hidden>
  • Date: Wed, 30 Apr 2008 08:08:04 -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

Attachment: smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________

Cocoa-dev mailing list (email@hidden)

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:

This email sent to email@hidden

References: 
 >Cache Class review (low priority) (From: Western Botanicals <email@hidden>)
 >Re: Cache Class review (low priority) (From: Jean-Daniel Dupas <email@hidden>)

  • Prev by Date: Re: How is "Apple + Ctrl + D" implemented?
  • Next by Date: Re: Icons on the NSTabview
  • Previous by thread: Re: Cache Class review (low priority)
  • Next by thread: Re: Cache Class review (low priority)
  • Index(es):
    • Date
    • Thread