[Scummvm-devel] Re: FilesystemNode specs

Max Horn max at quendi.de
Wed May 3 04:22:02 CEST 2006


Am 03.05.2006 um 12:28 schrieb Marcus Comstedt:

>
> Max Horn <max at quendi.de> writes:
[...]

>
> Personally, I don't think
>
> FSNode x = y.getChild("00.LFL");
> if(!x.isValid())
>   error;
> else if(x.isDirectory())
>   error;
> else if(!x.open())
>   error;
>
> is better than
>
> FSNode x = y.getChild("00.LFL");
> if(!x.open())
>   error;
>
> If there isn't any reason to handle the different kinds of errors in
> different ways, there is no reason why the client should have to do
> multiple checks.  It just makes it easier to forget to make one of the
> checks.

Aye. But that doesn't mean we have to merge File and FilesystemNode.  
Maybe we want to do that eventually, but if we do that, it'll require  
quite some changes to the existing codebase. So for now, I think it  
still should be:

FSNode x(y.getChildFile("00.LFL"));  // Note: Using copy constructor  
is faster
File z;
if (!z.open())
   error;

Whether we want to merge File and FSNode or not, we can discuss, but  
that's a quite different matter and involves other issues, so let's  
not mix those two things up for now. One step after the other :-).


>
> There should be a way for open() to give some indication as to what
> went wrong though.
yes. In particular, we could extend / replace Stream::ioFailed() to  
return an error code.

Also, if we remove the checking code, then listDir must be changed to  
return an error code (well, at least a bool indicating whether it  
failed or worked). I.e. change it from
	virtual FSList listDir(ListMode mode = kListDirectoriesOnly) const;
to
	virtual ErrorCode listDir(FSList &list, ListMode mode =  
kListDirectoriesOnly) const;


Cheers,
Max





More information about the Scummvm-devel mailing list