Re: NSTableView fix

[Tim McIntosh]

On Aug 29, 2007, at 6:00 PM, Andreas Höschler wrote:

> NSTableView
> Check whether delegate responds to  
> control:didFailToFormatString:errorDescription: before sending  
> message and a bunch of other important fixes.
>
> - (void)validateEditing
> {
>   if (_textObject)
>     {
>       NSFormatter *formatter;
>       NSString *string;
>       id newObjectValue = nil; // <----
>       BOOL validatedOK = YES;
>
>       formatter = [_editedCell formatter];
>       string = AUTORELEASE ([[_textObject text] copy]);
>
>       if (formatter == nil)
>         {
>           newObjectValue = string;
>         }
>       else
>         {
>           NSString *error;
>         
>           if ([formatter getObjectValue:&newObjectValue
>                          forString:string
>                          errorDescription: &error] == NO)
>             {
>              SEL sel = @selector 
> (control:didFailToFormatString:errorDescription:); // <-----
>              if ([_delegate respondsToSelector:sel] && [_delegate  
> control:self didFailToFormatString:string errorDescription:error]  
> == NO) // <----
>                {
>                 validatedOK = NO;
>                }
>              else
>                {
> //                newObjectValue = string; // <---- bad idea;  
> newObjectValue is expected to be a NSNumber, NSCalendarDate,...
>                }
>              if ([string length] > 0 && newObjectValue == nil)  
> validatedOK = NO; // <----
>             }
>         }
>       if (validatedOK == YES)
>         {
>           [_editedCell setObjectValue: newObjectValue];
>         
>           if (_dataSource_editable)
>             {
>               NSTableColumn *tb;
>         
>               tb = [_tableColumns objectAtIndex: _editedColumn];
>         
>               [self _setObjectValue: newObjectValue
>                     forTableColumn: tb
>                     row: _editedRow];
>
>               //[_dataSource tableView: self  setObjectValue: newObjectValue
>               //         forTableColumn: tb  row: _editedRow];
>             }
>         }
>     }
> }

I'm no expert in this area, but how about the alternative below?  The  
following functional differences are intentional; any others may be  
unintentional:

1. Doesn't assume newObjectValue is valid after  
getObjectValue:forString:errorDescription: returns NO.

2. Doesn't override delegate's decision in  
control:didFailToFormatString:errorDescription: when [string length]  
> 0.

3. Doesn't use [string length] == 0 as indicator of empty string  
(http://lists.apple.com/archives/cocoa-dev/2004/Jul/msg00166.html).

4. Doesn't use comparisons to Boolean constants (http://c-faq.com/ 
bool/bool2.html).

-Tim


- (void)validateEditing
{
  if (_textObject)
    {
      NSFormatter *formatter = [_editedCell formatter];
      NSString *string = AUTORELEASE ([[_textObject text] copy]);
      id newObjectValue = string;
      BOOL validatedOK = YES;

      if (formatter != nil)
        {
          NSString *error;

          validatedOK = [formatter getObjectValue:&newObjectValue
                                        forString:string
                                 errorDescription:&error];

          if (!validatedOK)
            {
              newObjectValue = nil;

              if ([_delegate respondsToSelector:@selector 
(control:didFailToFormatString:errorDescription:)])
                {
                  validatedOK = [_delegate control:self  
didFailToFormatString:string errorDescription:error];
                }
              else if ([string isEqualToString:@""])
                {
                  validatedOK = YES;
                }
            }
        }

      if (validatedOK)
        {
          [_editedCell setObjectValue: newObjectValue];
        
          if (_dataSource_editable)
            {
              NSTableColumn *tb;
        
              tb = [_tableColumns objectAtIndex: _editedColumn];
        
              [self _setObjectValue: newObjectValue
                    forTableColumn: tb
                    row: _editedRow];
            }
        }
    }
}