Would anyone mind......

Discussion in 'Mac Programming' started by mdeh, Feb 18, 2009.

  1. macrumors 6502

    Joined:
    Jan 3, 2009
    #1
    Looking at this code and offer some suggestions/criticism.

    Note that GC is not yet covered. It's an answer to exercise 16.1 as follows.

    Assumes that the files and directory reside in the current working directory.

    Thanks in advance.

    Code:
    #import <Foundation/NSObject.h>
    #import <Foundation/NSFileManager.h>
    #import <Foundation/NSString.h>
    #import <Foundation/NSProcessInfo.h>
    #import <Foundation/NSEnumerator.h>
    #import <Foundation/NSAutoreleasePool.h>
    
    #define		ONEARGUMENT		  1
    
    
    
    int main (int argc, const char * argv[]) {
        NSAutoreleasePool			* pool = [[NSAutoreleasePool alloc] init];
    	NSFileManager				*fm = [NSFileManager defaultManager];
    	NSProcessInfo				*proc = [NSProcessInfo processInfo];
    	NSArray						*args = [proc arguments];
    	BOOL						isDirectory = NO, exists = NO;
    	NSString					*working_directory, *destDir, *srcfile, *destfile;
    	NSInteger					argCount;
    	
    	NSLog(@"Current directory: %@", [fm currentDirectoryPath]);
    	
    	/* check for correct usage */
    	if ( [[proc arguments] count] == ONEARGUMENT )
    	{
    		NSLog(@"Usage: file1, file2..., prog");
    		return 1;
    	}
    	
    	/*check last argument is a directory */
    	working_directory = [ fm currentDirectoryPath];
    	destDir = [working_directory stringByAppendingPathComponent: [[proc arguments]lastObject ]];
    	exists =  [ fm fileExistsAtPath: destDir isDirectory: &isDirectory];
    	if (!exists )
    	{
    		NSLog(@"Usage:  Directory does not exist");
    		return 2;
    	}
    	
    	if ( !isDirectory )
    	{
    		NSLog(@"Usage: Last file must be a Directory");
    		return 3;
    	}
    	
    	/* iterate through[proc arguments] */
    	
    	argCount = args.count;
    	int fileCount = 1;
    	
    	
    	while (--argCount > 1)
    	{
    		srcfile = [args objectAtIndex: fileCount++];
    		
    		/*is source file readable */
    		
    		if ( [ fm isReadableFileAtPath: srcfile] == NO)
    		{
    			NSLog(@"Error: Source file %@ is unreadable", srcfile);
    			return 4;
    		}
    		
    		/*create destination path */
    		
    		destfile = [ destDir stringByAppendingPathComponent: srcfile];
    		/* remove file if exists at destination */
    		[fm removeItemAtPath:destfile error:nil];
    		if ([fm copyPath:srcfile toPath:destfile handler:nil] == NO)
    		{
    			NSLog(@"Error: copy failed" );
    			return 5;
    		}
    		
    		NSLog(@"Copy of %@ to %@ succeeded", srcfile, destfile);
    		NSLog(@"Contents of source:%@", [NSString stringWithContentsOfFile:srcfile encoding:NSUTF8StringEncoding error:nil]); 
    		NSLog(@"Contents of destination:%@",[NSString stringWithContentsOfFile:destfile encoding:NSUTF8StringEncoding error:nil]);
    	}
    	
      
        [pool drain];
        return 0;
    }
     
  2. macrumors 601

    HiRez

    Joined:
    Jan 6, 2004
    Location:
    Western US
    #2
    Don't have time to look it over in detail, but it looks really good initially. Appears you are checking for all the right things, in the right order, providing good feedback, and your code is well formatted and commented. So good job! I assume you've actually run it and it works? Also, maybe you want to prompt to overwrite existing files, or make that an option flag?
     
  3. macrumors 68040

    lee1210

    Joined:
    Jan 10, 2005
    Location:
    Dallas, TX
    #3
    My least favorite lines are these:
    Code:
    while (--argCount > 1)
    	{
    		srcfile = [args objectAtIndex: fileCount++];
    
    It seems like this should work fine, but is a little difficult to follow. I don't know if the book has covered fast enumeration or not yet, but i would modify args by removing the first and last element and either store it back in args or assign it to a new NSArray called fileList or something like that. Then you could just run over fileList using fast enumeration or a simple for loop from 0 to [fileList count]-1, etc. The loop would be cleaner, and there would only be one thing you're "counting", which would be the current position (w/ Fast Enumeration, there'd be nothing to count at all).

    The only other thing i am not so sure about is memory management. I don't know if all of the routines used return autoreleased objects. For a program this small it probably isn't a big deal, but i didn't see any retain/release/autorelease messages, so you might just want to be sure you're being tidy. Not for practical reasons for this program, but just to get in the habit.

    -Lee

    Edit: the fast enumeration documentation is here, if you're interested:
    http://developer.apple.com/DOCUMENT.../ObjectiveC/Articles/chapter_8_section_1.html

    To get a new array without a few elements, you could use something like objectsAtIndexes:
    http://developer.apple.com/DOCUMENT...apple_ref/occ/instm/NSArray/objectsAtIndexes:
    or subarrayWithRange:
    http://developer.apple.com/DOCUMENT...pple_ref/occ/instm/NSArray/subarrayWithRange:

    Edit 2: This post was critical, but the intent was "Everything looks good. The only thing i'd change would be..."
     
  4. thread starter macrumors 6502

    Joined:
    Jan 3, 2009
    #4
    Thank you Hirez...and today I did go outside...and it was wonderful :)
     
  5. thread starter macrumors 6502

    Joined:
    Jan 3, 2009
    #5
    Lee..yes..fast enumeration is covered. I **really** like your idea of a sub-array which is then enumerated. I always had problems making sure the x-- worked with the ptr++ :) ...even though when it does work, you feel like a real programmer!!!
    Will implement your changes...very slick!
    What criticism?
    Thank you as always.
     
  6. thread starter macrumors 6502

    Joined:
    Jan 3, 2009
    #6

    What about this ( much thanks to you)

    Code:
    /* create subarray that will be enumerated */
    	subArrRnge.location = 1; /* first argument after program name */
    	subArrRnge.length = args.count - 2; /* 1  for program name and 1 for directory */
    	
    	NSArray	*subArr = [ args subarrayWithRange: subArrRnge];
    	
    	
    	/* iterate through subArr */
    	
    	for ( srcfile in subArr)	
    	
    	{ ..................
    
    
    where args = [proc arguments].

    Lee...it is **so** much cleaner...I am beginning to like Obj-C.

    thanks so much again.
     
  7. macrumors 68040

    lee1210

    Joined:
    Jan 10, 2005
    Location:
    Dallas, TX
    #7
    No problem. It is a little longer, but definitely easier read.

    This is a *little* more convoluted, and the NSMakeRange could be moved to a new line, but it saves a little space. I don't really love mixing a function call in with a message pass... But anywho, here it is:
    Code:
    NSArray *subArr = [args subarrayWithRange: NSMakeRange(1,args.count - 2)];
    -Lee
     
  8. thread starter macrumors 6502

    Joined:
    Jan 3, 2009
    #8
    Very nice...I will incorporate that too...appeals to the C side of me!!! Thank you again.
     

Share This Page