On Jul 9, 2008, at 10:44 AM, Chris Paveglio wrote:
I'm trying to make sure that this part of my code is good and clean
and not leaking. I believe that I have it right (but still think I
have something wrong).
This code makes a list of file paths, and then copies the files or
folders from their location to the destination the user selected. If
the item already exists in the destination, I delete it first (to
overwrite with new data, yes this is what I intend and the user is
aware of how the app works).
I've read all your previous responses (Thanks) and it seems like
since I am just using "normal" strings that they will be
autoreleased on their own. There are now no compiler cautions and
the app seems to work OK.
A side issue is that I want to add a 2 second delay before closing
the sheet that is displayed, is NSTimer the correct item to use?
Chris
I have some stylistic comments on this code.
Your method is too large. To make it smaller you should break it up
into a number of smaller tasks.
As Jens mentioned a string like this
NSString *string1 = @"Library/Preferences/Adobe Photoshop CS3
Settings";
is pretty much the same as a string like this: @"Library/Preferences/
Adobe Photoshop CS3 Settings"
I would build a method that does nothing but return the array or
arrays that contain your manifest of files to save
-(NSArray)myPrefsArray
{
// build array here
return myPrefs;
}
I notice that your myPrefsSaved array is derived from the myPrefs
array but it just has the file name not the full path. I'd use
[astring lastPathComponent] to either build the second array or just
call this method inside your loop.
I would break out the entire copy files loop into its own method.
Since you loop over the contents of the myPrefs array I wouldn't hard
code the loop index to 8 or 9 I'd depend on the [myrefsArray length]
value to index the loop. You should also look at NSEnumerator and
also fast enumeration as ways to drive this loop in a better way.
I notice that you call [myPrefs objectAtIndex:i] several times in your
loop. If you use NSEnumerator or fast enumeration you can avoid this
otherwise just call it once at the top of the loop and save the value
in a temp string variable.
Also, declaring the loop counter outside the loop is now old
fashioned. You can try this:
for (int i = 0; i < limit; i++)
You might need to set your C dialect setting to C99 or gnu99 if you're
using an old project. I recommend it.
You don't check any errors from the file manager operations. What if
the user chooses a folder on a locked volume or the disk gets full or
the user chooses a network volume and the network drops out? Your
users will hate you if these things happen but your app fails
silently. You should both check for errors and report them to the user.
I don't see any problems in this code with leaking of objects.
I don't like the way that all the steps of this process are done one
after another in a sequential fashion. I would probably set up the
tasks to be done in a separate thread or have each file copy be done
inside a timer callback until all are done and show a progress window
probably in place of a progress sheet (although it might work ok
either way).
Regarding your 2 second delay you can use a timer or
performSelector:withObject:afterDelay but both will require you to
provide a callback method that will dismiss your sheet.
Good luck,
Brian
_______________________________________________
Cocoa-dev mailing list (Cocoa-dev@lists.apple.com)
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]
