Richard Frith-Macdonald wrote:
> Nicola Pero wrote:
>
>> Ok - it looks silly to discuss too much about this anyway (with all the
>> pending stuff) but I can't avoid another one on this :-)
>>
>>
>>
>>>> I don't understand why you are assuming that +stringWithContentsOfFile
>>>> returning nil is an error. The caller will assess that. In the case of
>>>> NSSavePanel, for example, it's a perfectly valid condition.
>>>>
>>>>
>>> <pedantic>Technically a nil return here indicates a failure ... an
>>> error. I'ts up to the caller to decide how to deal with that, not to
>>> decide whether it's a failure or not.</pedantic>
>>>
>>
>> <pedantic>An 'error', for the programmer using the base library, is when
>> the base library generates an exception. Anything else is not an
>> error. A nil return here indicates that the file doesn't exist, is
>> not readable
>> or can't be read. This is considered a valid result, otherwise the API
>> would have required an exception - with the name and reason of the
>> error -
>> to be raised.</pedantic>
>>
>
> Sorry ... but the above is simply not the case. Exceptions are not the
> only way of denoting
> an error condition and the standard mechanism of indicating an error
> during object
> construction is to return nil. As a general rule, constructors are
> supposed to avoid
> raising exceptions, and should document cases where they do since
> raising an exception
> is not the normal behavior for them.
>
>> But I agree that it's up to the caller to decide how to deal with a nil
>> result, which is why the library should not be logging but letting the
>> caller decide what to do. :-)
>>
>> Yes - it's a very simple (but not very powerful) API ... the idea I
>> suppose is that if you want richer control, then you have to use a more
>> complex API.
>>
>> +stringWithContentsOfFile: is a simple high-level wrapper around the
>> low-level API, which provides you with a simple and straightforward API
>> for reading a file if it's there, and ignore it if it's not or can't be
>> read. It's perfect for cases like the NSSavePanel, which need precisely
>> that. The fact that it returns nil is to be considered a *feature* of
>> the
>> library, and using it is *not* a programming error.
>>
> Returning a nil is a failure indicator ... sure you can consider it a
> 'feature', but
> that's not really the point - the log is a warning - to indicate
> something
> that is likely to be a programing error. By 'likely' I mean that it's
> a programming
> error often enought to be worth logging ... if it was 'always' a
> programming error
> we would either be writing an NSLog() (no choice about it), or raising
> an NSException.
> An NSLog() would be used where we know it's an error but expect the
> program to
> be able to continue without intervention anyway, thoough we might also
> use it
> where we are fairly certain it's a error.
>
> I've already said that elsewhere, but perhaps restating will help.
> It's common practice
> to log possible problems both as an aid to development and for
> diagnostic purposes later.
> Under Unix, NeXTstep (and now MacOS-X) we commonly log things to
> stderr. In NeXT/MacOS/GNUstep gui applications, the naive user never
> sees these logs ...
> so they don't bother the user ...
> but they do provide useful information for programmers. The reason for
> not
> logging *loads* of stuff is for performance, and so that more serious
> problems
> don't get lost amidst all the very routine stuff.
>
> So the 'hierarchy' of logging is like this -
>
> NSDebugLog() ... interesting circumstances, possibly errors, but not
> likely to be of
> use logging under normal conditions. Needs to be turned on to occur.
> NSWarnLog() ... likely to be an error, almost always worth logging.
> Needs to be turned off
> if it's not to occur.
> NSLog() ... probable error, always considered worth logging. can't be
> turned off
> NSException ... definite error requiring some intervention if the
> program is to continue
> running. Results in log and abort otherwise.
>
> Error handling is a different issue, and is handled by two mechanisms
> ... return values
> from functions and methods (IMO best used to denote errors which
> should be dealt
> with immediately in the code), and exceptions (best used to pass error
> information
> back up the stack to where the program can deal with them in a
> different manner).
>
>>
>>
>> Good explanation - but then why the following code -
>>
>> tmp = NSZoneMalloc(zone, fileLength);
>> if (tmp == 0)
>> {
>> NSWarnFLog(@"Malloc failed for file (%s) of length %d - %s",
>> thePath, fileLength, GSLastErrorStr(errno));
>> CloseHandle(fh);
>> return NO;
>> }
>>
>> is this an error that the programmer has done when calling the method ?
>>
>
> No, that's just a mistake :-)
> I think a memory allocation failure causes the program to abort
> anyway, and
> the log there would never get executed.
>
>> The
>>
>> explanation of why you are using NSWarnLog is not very convincing.
>> You are checking the result of each operation you do, and producing a
>> NSWarnLog is any operation fails. But it's not the programmer's fault if
>> memory can't be allocated, or if a file was deleted by another process
>> just after the method was called (but before the file was opened), or
>> its
>> permissions changed in the same way, or if the file is too big or if an
>> fseek on the file fails (or if the NFS went down just after the method
>> started executing).
>>
> That's just because some of them should actually be NSlog() or
> NSException
> cases because they are more severe problems ... and I went through
> quickly to
> *improve* the situation, rather than spending time trying to perfect it.
>
>> provided
>> This stuff is not in the programmer's control, and not the programmers's
>> fault, and a different programming style would not necessarily remove
>> the
>> cause of the errors - so it should not be a NSWarnLog. Why are you
>> against it being a NSDebugLog ?
>>
>>
> Because warn logging should be on by default ... and only (possibly)
> turned off for
> a 'production' system where you don't want end users to see any of
> this if they start
> poking around places where they are not expected.
>
> Why do you want to turn *off* useful information?
>
>> By the way, it seems you want to always have the calling code separately
>> check if the the file is there (and that it's not a dir), then check if
>> the file is readable by you, and only then read it using
>> +stringWithContentsOfFile:.
>>
>> Well in the NSSavePanel I don't see how that would help or be a better
>> programming style ... it would require us to write a lot more code in
>> NSSavePanel to do exactly the same things - read the file if it's there
>> and can be read as a file, and ignore the matter in all other cases.
>>
>> It would also be less efficient, since it would check twice that the
>> file
>> is readable (basically part of the +stringWithContentsOfFile: would need
>> to be duplicated in the caller, which *is* bad programming style and
>> cause
>> of bugs).
>>
>>
>
> It lists the files in a directory, then removes certain names from the
> list.
> If the .hidden file is not in the list, it does not need to try
> reading it.
> Even if it couldn't reasonably work that way for some reason, I'd
> advocate using -
> if ([mgr isReadableFileAtPath: hidden] == YES)
> // Read hidden file and do whatever it is we do.
>
> Certainly there is no cause to duplicate any stringWithContentsOfFile:
> code though.
>
> But in any case, I'm not saying you should definitely not use
> 'features' of the API
> in ways for which they might not have been intended ... just that you
> should be
> aware of warnings/logs if you do.
>
>>
>>
>> There are millions of reasons why a read from a file can fail without
>> any
>> programming error - no more memory, NFS server down, NFS network
>> problems,
>> file removed, permissions changed, cdrom/floppy unmounted, filesystem
>> error, harddisk failure, floppy failure, cdrom damaged ... any of these
>> can happen at any time; and conditions can very well change between the
>> moment that the programmer checks that the file is readable and the
>> moment
>> that he reads it. Not so uncommon.
>>
>>
>
> Not *SO* uncommon ... but still all those conditions form a tiny
> minority of
> cases. The vast majority of cases in practice are where the programmer
> has
> provided the name of a file which does not exist or is not accessible for
> chmod/chown reasons. And of that vast majority, a substantial minority
> are cases where the wrong file name has been given ... genuine
> programming
> errors by many standard. It's therefore helpful to have a log for
> these, and it's
> a good thing for programmers to try to avoid them.
>
>> I can see that some people might want a NSDebugLog of what's
>> happening to
>> help in debugging weird problems. But it has nothing to do with
>> programming errors or programming practice or teaching people how to
>> improve/fix their code.
>>
>
> As I said ... ten years ago I'd probably have said that conflating the
> two operations
> of checking for a file and reading a file made sense for
> convenience... since then
> I've realised that I make more mistakes than I thought, and many
> people make more
> mistakes than me - So I now think that checking first usually makes
> sense, and anything
> that raises peoples awareness of this is probably a good thing.
>
>>
>>
>>
>>
>>> and we ought to let the programmer know so they can improve/fix
>>> their code.
>>>
>>
>> The NSSavePanel code is correct, is using the documented API, exactly as
>> documented, and efficiently and elegantly - I see no reason to change
>> it.
>>
> I'm not going to dispute that ... much :-)
> The code could be made more efficient by not reading the .hidden file
> if it does not
> exist ... I think whether you consider this more or less elegant is a
> matter of taste.
>
PS.
I know warn logs are selectable at base library compile time ... I'd
have no problem
with (I even like the idea) of having a flag to deselect them at runtime.
eg. --GNU-Debug=NoWarn to turn off warnings.
Worthwhile?
_______________________________________________
Bug-gnustep mailing list
[EMAIL PROTECTED]
http://mail.gnu.org/mailman/listinfo/bug-gnustep
