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]
