NSDocument window opens - even if the document didn't read properly.

  • I hope that someone here might be able to help me with a couple of queries.

    1. I'm trying to open a document (NSDocument).  If the file is good then my program opens it without problem.  If the document fails to parse correctly then the NSDocument window still opens - but it opens empty.  Of course, if parsing of the document fails then I want the document window not to open at all.  The document data gets read as so and if it fails then I return NO - the document window still opens though.  Can anyone suggest what I might have missed or messed up?

    - (BOOL)readFromData:(NSData *)data ofType:(NSString *)typeName error:(NSError **)outError
    {
        BOOL success = NO;

        success = [self loadTextViewWithInitialData: data];
        if (!success)
        {
            NSArray* paths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
            NSString* URLString = [[[paths objectAtIndex:0] stringByAppendingString:[self fileURL].lastPathComponent]stringByAppendingString:@".bad"];
            NSURL* destinationURL =  [NSURL URLWithString:[URLString stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding]];
            BOOL success = [[NSFileManager defaultManager] copyItemAtURL:[self fileURL] toURL:destinationURL error:nil];
        }

        return success;
    }

    2.  If the document fails to open then I want to copy it to a new location, with a new extension (see above).  This fails.  I can't think why!
  • Hello, there are many things wrong with your code. I’m noting them below.

    On 29 Jul 2012, at 18:53, Pascal Harris wrote:

    > I hope that someone here might be able to help me with a couple of queries.
    >
    > 1. I'm trying to open a document (NSDocument).  If the file is good then my program opens it without problem.  If the document fails to parse correctly then the NSDocument window still opens - but it opens empty.  Of course, if parsing of the document fails then I want the document window not to open at all.  The document data gets read as so and if it fails then I return NO - the document window still opens though.  Can anyone suggest what I might have missed or messed up?
    >
    > - (BOOL)readFromData:(NSData *)data ofType:(NSString *)typeName error:(NSError **)outError
    > {
    > BOOL success = NO;
    >
    > success = [self loadTextViewWithInitialData: data];

    Uh-oh, if this call failed, you’re returning NO without filling in the error pointer. This will either blow up or provide a poor message to the user. Hopefully your -loadTextView… method can report why it failed.

    > if (!success)
    > {
    > NSArray* paths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);

    Cocoa has URL-based methods for this in NSFileManager these days; you should consider adopting them.

    > NSString* URLString = [[[paths objectAtIndex:0] stringByAppendingString:[self fileURL].lastPathComponent]stringByAppendingString:@".bad"];

    This is some seriously mangled code. Cocoa provides path manipulation-specific string routines. USE THEM.

    > NSURL* destinationURL =  [NSURL URLWithString:[URLString stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding]];

    This is NOT how you convert from a path to a URL. Use +fileURLWithPath: instead.

    > BOOL success = [[NSFileManager defaultManager] copyItemAtURL:[self fileURL] toURL:destinationURL error:nil];

    You’ve just stored success into a *new* variable. I’m guessing you wanted to store it into the existing success variable. Unless you’ve changed the default compiler settings, the compiler should be warning you here that the success variable is unused. Compiler warnings are useful. HEED THEM.

    > }
    >
    > return success;
    > }
  • Mike,

    Thanks for taking time on a Sunday to reply promptly.  I'm very grateful.  Taking each point

    On 29 Jul 2012, at 19:10, Mike Abdullah <cocoadev...> wrote:

    > Hello, there are many things wrong with your code. I’m noting them below.
    >
    > On 29 Jul 2012, at 18:53, Pascal Harris wrote:
    >
    >> I hope that someone here might be able to help me with a couple of queries.
    >>
    >> 1. I'm trying to open a document (NSDocument).  If the file is good then my program opens it without problem.  If the document fails to parse correctly then the NSDocument window still opens - but it opens empty.  Of course, if parsing of the document fails then I want the document window not to open at all.  The document data gets read as so and if it fails then I return NO - the document window still opens though.  Can anyone suggest what I might have missed or messed up?
    >>
    >> - (BOOL)readFromData:(NSData *)data ofType:(NSString *)typeName error:(NSError **)outError
    >> {
    >> BOOL success = NO;
    >>
    >> success = [self loadTextViewWithInitialData: data];
    >
    > Uh-oh, if this call failed, you’re returning NO without filling in the error pointer. This will either blow up or provide a poor message to the user. Hopefully your -loadTextView… method can report why it failed.
    Good point - I've now fixed this.  That still doesn't explain why the window stays open though.  When I fix this problem, it doesn't provide an error message to the user either.  Perhaps this is connected to the reason that the window opens anyway, even if the data can't be read.

            NSMutableDictionary *errorDetail = [NSMutableDictionary dictionary];
            [errorDetail setValue:@"This file is corrupted and could not be read." forKey:NSLocalizedDescriptionKey];

            *outError = [NSError errorWithDomain:NSOSStatusErrorDomain code:1 userInfo:
                    errorDetail];

    >
    >> if (!success)
    >> {
    >> NSArray* paths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
    >
    > Cocoa has URL-based methods for this in NSFileManager these days; you should consider adopting them.
    Thank you - I will.  But I want to get this working first.  By examining the path provided in the string, I can see that the destination is valid - and I've used code like this successfully in the past.  Is the problem that the file being copied is also the file that I'm attempting (unsuccessfully) to open?  Is it that there's a temporary lock on the file that prevents it from copying successfully?

    >
    >> NSString* URLString = [[[paths objectAtIndex:0] stringByAppendingString:[self fileURL].lastPathComponent]stringByAppendingString:@".bad"];
    >
    > This is some seriously mangled code. Cocoa provides path manipulation-specific string routines. USE THEM.
    >
    >> NSURL* destinationURL =  [NSURL URLWithString:[URLString stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding]];
    >
    > This is NOT how you convert from a path to a URL. Use +fileURLWithPath: instead.
    Again, it's worked in the past.  I changed it to fileURLWithPath - but the end result is the same.

    >
    >> BOOL success = [[NSFileManager defaultManager] copyItemAtURL:[self fileURL] toURL:destinationURL error:nil];
    >
    > You’ve just stored success into a *new* variable. I’m guessing you wanted to store it into the existing success variable. Unless you’ve changed the default compiler settings, the compiler should be warning you here that the success variable is unused. Compiler warnings are useful. HEED THEM.
    I do use the variable - just not in that little example I gave.  And rest assured that I heed all compiler warnings, and deal with them.
  • On Jul 29, 2012, at 10:53 , Pascal Harris <45rpmlists...> wrote:

    > 2.  If the document fails to open then I want to copy it to a new location, with a new extension (see above).  This fails.  I can't think why!

    On Jul 29, 2012, at 11:47 , Pascal Harris <45rpmlists...> wrote:

    > Good point - I've now fixed this.  That still doesn't explain why the window stays open though.  When I fix this problem, it doesn't provide an error message to the user either.  Perhaps this is connected to the reason that the window opens anyway, even if the data can't be read.

    The implication of your two statements here (no error message, no creation of the copy) is that 'loadTextViewWithInitialData' is in fact returning YES.

    Returning NO from 'readFromData:' really does cause NSDocumentController to display an alert (except when the returned error is NSCocoaErrorDomain/NSUserCancelledError, which is a standard way of preventing it from complaining). Since you've already acknowledged that the code you posted is not the actual code, we have to assume you have a code path through your real code that's returning YES.

    > NSMutableDictionary *errorDetail = [NSMutableDictionary dictionary];
    > [errorDetail setValue:@"This file is corrupted and could not be read." forKey:NSLocalizedDescriptionKey];
    >
    > *outError = [NSError errorWithDomain:NSOSStatusErrorDomain code:1 userInfo:
    > errorDetail];

    You should always check that 'outError' isn't NULL before dereferencing it. The NSError** outError pattern doesn't require outError to be non-NULL. I suspect that NSDocumentController never passes NULL, but it would be best to get used to avoiding the assumption.
  • On 29 Jul 2012, at 19:47, Pascal Harris wrote:

    > Mike,
    >
    > Thanks for taking time on a Sunday to reply promptly.  I'm very grateful.  Taking each point
    >
    > On 29 Jul 2012, at 19:10, Mike Abdullah <cocoadev...> wrote:
    >
    >> Hello, there are many things wrong with your code. I’m noting them below.
    >>
    >> On 29 Jul 2012, at 18:53, Pascal Harris wrote:
    >>
    >>> I hope that someone here might be able to help me with a couple of queries.
    >>>
    >>> 1. I'm trying to open a document (NSDocument).  If the file is good then my program opens it without problem.  If the document fails to parse correctly then the NSDocument window still opens - but it opens empty.  Of course, if parsing of the document fails then I want the document window not to open at all.  The document data gets read as so and if it fails then I return NO - the document window still opens though.  Can anyone suggest what I might have missed or messed up?
    >>>
    >>> - (BOOL)readFromData:(NSData *)data ofType:(NSString *)typeName error:(NSError **)outError
    >>> {
    >>> BOOL success = NO;
    >>>
    >>> success = [self loadTextViewWithInitialData: data];
    >>
    >> Uh-oh, if this call failed, you’re returning NO without filling in the error pointer. This will either blow up or provide a poor message to the user. Hopefully your -loadTextView… method can report why it failed.
    > Good point - I've now fixed this.  That still doesn't explain why the window stays open though.  When I fix this problem, it doesn't provide an error message to the user either.  Perhaps this is connected to the reason that the window opens anyway, even if the data can't be read.
    >
    > NSMutableDictionary *errorDetail = [NSMutableDictionary dictionary];
    > [errorDetail setValue:@"This file is corrupted and could not be read." forKey:NSLocalizedDescriptionKey];
    >
    > *outError = [NSError errorWithDomain:NSOSStatusErrorDomain code:1 userInfo:
    > errorDetail];

    As Quincey points out, you should check the outError pointer first before filling it in. Also, we already have NSFileReadCorruptFileError which would seem the most appropriate error code here.
    >
    >
    >>
    >>> if (!success)
    >>> {
    >>> NSArray* paths = NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES);
    >>
    >> Cocoa has URL-based methods for this in NSFileManager these days; you should consider adopting them.
    > Thank you - I will.  But I want to get this working first.  By examining the path provided in the string, I can see that the destination is valid - and I've used code like this successfully in the past.  Is the problem that the file being copied is also the file that I'm attempting (unsuccessfully) to open?  Is it that there's a temporary lock on the file that prevents it from copying successfully?

    Unlikely.
    >
    >
    >>
    >>> NSString* URLString = [[[paths objectAtIndex:0] stringByAppendingString:[self fileURL].lastPathComponent]stringByAppendingString:@".bad"];
    >>
    >> This is some seriously mangled code. Cocoa provides path manipulation-specific string routines. USE THEM.
    >>
    >>> NSURL* destinationURL =  [NSURL URLWithString:[URLString stringByAddingPercentEscapesUsingEncoding:NSUTF8StringEncoding]];
    >>
    >> This is NOT how you convert from a path to a URL. Use +fileURLWithPath: instead.
    > Again, it's worked in the past.  I changed it to fileURLWithPath - but the end result is the same.
    If it worked in the past, the strings you had were NOT file paths. Just because a URL object is created does not necessarily mean it’s the one you’re expecting.
    >
    >>
    >>> BOOL success = [[NSFileManager defaultManager] copyItemAtURL:[self fileURL] toURL:destinationURL error:nil];
    >>
    >> You’ve just stored success into a *new* variable. I’m guessing you wanted to store it into the existing success variable. Unless you’ve changed the default compiler settings, the compiler should be warning you here that the success variable is unused. Compiler warnings are useful. HEED THEM.
    > I do use the variable - just not in that little example I gave.  And rest assured that I heed all compiler warnings, and deal with them.

    Alright, show us the real code then! It’s impossible for us to debug code we can’t see.

    Also I forgot to mention that since -copyItem… can return an error, you should make use of that in your routine.
previous month july 2012 next month
MTWTFSS
            1
2 3 4 5 6 7 8
9 10 11 12 13 14 15
16 17 18 19 20 21 22
23 24 25 26 27 28 29
30 31          
Go to today