Re: Does this caution need fixed? (newb)
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 #import MyExporter.h @implementation MyExporter - (IBAction)exportPrefs:(id)sender { NSOpenPanel *theOpenPanel = [NSOpenPanel openPanel]; [theOpenPanel setTitle:@Choose a folder to save your prefs into:]; [theOpenPanel setCanChooseDirectories:YES]; [theOpenPanel setCanChooseFiles:NO]; if ([theOpenPanel runModal] == NSOKButton) //if user chooses a folder { NSString *theDestination = [theOpenPanel filename]; //get name NSFileManager *theManager = [NSFileManager defaultManager]; //make an NSFM //if files dont exist on user machine it just seems to bypass copying them NSString *string1 = @Library/Preferences/Adobe Photoshop CS3 Settings; NSString *string1a = @Adobe Photoshop CS3 Settings; NSString *string2 = @Library/Preferences/Adobe Illustrator CS3 Settings; NSString *string2a = @Adobe Illustrator CS3 Settings; NSString *string3 = @Library/Preferences/Adobe InDesign/Version 5.0; NSString *string3a = @Version 5.0; NSString *string4 = @Library/Preferences/CDFinder Preferences; NSString *string4a = @CDFinder Preferences; NSString *string5 = @Library/Application Support/Firefox; NSString *string5a = @Firefox; NSString *string6 = @Library/Safari; NSString *string6a = @Safari; NSString *string7 = @Library/Application Support/Adobe/Adobe PDF/Settings; NSString *string7a = @Settings; NSString *string8 = @Library/Preferences/Acrobat Distiller Prefs; NSString *string8a = @Acrobat Distiller Prefs; NSString *string9 = @Documents/Microsoft User Data; NSString *string9a = @Microsoft User Data; //make 2 arrays with locations of orig prefs, and then short version to save in folder NSArray *myPrefs; myPrefs = [NSArray arrayWithObjects: string1, string2, string3, string4, string5, string6, string7, string8, string9, nil]; NSArray *myPrefsSaved; myPrefsSaved = [NSArray arrayWithObjects: string1a, string2a, string3a, string4a, string5a, string6a, string7a, string8a, string9a, nil]; //start panel here [NSApp beginSheet:exportSheet modalForWindow:mainWindow modalDelegate:self didEndSelector:NULL contextInfo:nil]; [exportProgressBar setDoubleValue:(double)3.0]; //fake ourselves some progress [exportProgressBar displayIfNeeded]; int i; for (i = 0; i 8; i++) //skip string9 for testing, takes too long to copy { NSString *theSettings = [NSHomeDirectory() stringByAppendingPathComponent:[myPrefs objectAtIndex:i]]; //change/set out variable to this data //loop with each item saving into our destination folder NSString *theDestinationFile = [theDestination stringByAppendingPathComponent:[myPrefsSaved objectAtIndex:i]]; [exportCurrentFileName setStringValue:(@%@,[myPrefsSaved objectAtIndex:i])]; [exportCurrentFileName displayIfNeeded]; //give a kick to our text box dammit! //delete the old pref file [theManager removeFileAtPath:theDestinationFile handler:nil]; //copy the file [theManager copyPath:theSettings toPath:theDestinationFile handler:nil]; [exportProgressBar incrementBy:(double)11.0]; //11 is arbitrary progress (100 divided by 9 items) [exportProgressBar displayIfNeeded]; } [exportProgressBar setDoubleValue:100.0];
Re: Does this caution need fixed? (newb)
On 9 Jul '08, at 7: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). It looks OK to me on quick reading. The main thing I'd change is to put the path strings into a plist, or at least put the string literals directly into the NSArray initializer lines instead of declaring separate variables for them, i.e. myPrefs = [NSArray arrayWithObjects: @Library/Preferences/Adobe Photoshop CS3 Settings, @Adobe Photoshop CS3 Settings, ..., nil]; but that's just an issue of style. —Jens smime.p7s Description: S/MIME cryptographic signature ___ 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]
Re: Does this caution need fixed? (newb)
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]
Re: Does this caution need fixed? (newb)
On Thu, Jul 3, 2008 at 1:47 PM, Kyle Sluder [EMAIL PROTECTED] wrote: Many (dare I say most?) developers consider warnings to be the equivalent of the compiler vomiting in its mouth -- errors are the subsequent suffocation. Perhaps you can tell how strongly I feel about this. I completely agree with your opinion about warnings, but... ewww! :-) sherm-- -- Cocoa programming in Perl: http://camelbones.sourceforge.net ___ 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]
Re: Does this caution need fixed? (newb)
Chris Paveglio wrote: My code is like this: NSMutableString *theSettings; theSettings = [[NSMutableString alloc] init]; //myPrefs is an array of strings, each item is like Library/Safari int i; for (i = 0; i 8; i++ { theSettings = [NSHomeDirectory() stringByAppendingPathComponent:[myPrefs objectAtIndex:i]]; } Thinking about it, do I need the alloc and init commands? Sometimes I am unsure about what needs alloc or init versus what I can just declare as a variable without doing that. No, you don't. Also, unless you're going to modify settings after assigning it in your loop, then you should make it a NSString *. The compiler is likely complaining because you're assigning NSString * to a NSMutableString *. If you really need a mutable string then change the assignment line to this: theSettings = [[NSHomeDirectory() stringByAppendingPathComponent:[myPrefs objectAtIndex:i]] mutableCopy]; HtH, Jason ___ 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]
Re: Does this caution need fixed? (newb)
On Jul 3, 2008, at 9:40 AM, Chris Paveglio wrote: Also, should my code be caution free as a sign of clean coding or can some cautions that don't affect functionality be dismissed? I'm one of those people who turns on just about every warning and then fixes the code that generates the warnings. Yeah, I might need to do more work up front, but I'd rather know what the compiler is thinking than have to later suffer a hard-to-track bug due to a case of late-night coding... :) steve ___ 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]
Re: Does this caution need fixed? (newb)
On 7/3/08 9:40 AM, Chris Paveglio said: I have a loop that gets the user's home directory, and then adds a string to complete the file path for several files. This line: theSettings = [NSHomeDirectory() stringByAppendingPathComponent:[myPrefs objectAtIndex:i]]; gives me a caution sign when I compile In addition to the other good comments, you should be extra extra careful when writing file-related code: you don't want to accidently move, copy, delete at the wrong path. So I would add Ia test to check for nil being returned from NSHomeDirectory(). -- Sean McBride, B. Eng [EMAIL PROTECTED] Rogue Researchwww.rogue-research.com Mac Software Developer Montréal, Québec, Canada ___ 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]
Re: Does this caution need fixed? (newb)
Chris Paveglio wrote: Thanks all for your help and insight! I believe Jason's solution will work for me as I am changing the assignment of what theSetting is each time through the loop. I have a list (array) of files that gets copied from one place to the other, and I change the origin and the destination each time in the loop. Also Michael I understand now what you are saying about the assignment. Thank you for putting it in a nice detailed explanation! :-) Just keep in mind that you'll be leaking memory with my example if you use it as written. I'm assuming that you'll do something with the string inside the loop. After doing whatever it is you do with it, you'll need to release theSettings. Making a mutable copy does allocate memory for the copy. ___ 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]