[Clam-devel] PATCH: new File class heirarchy

[Zach Welch]

On Tuesday 10 April 2007 09:13, Xavier Amatriain wrote:
> Pau, all, I think there are at least 3 separate issues that need to be
> discussed independently:
>
> 1. Is there a need to refactor into a new File class?
> 2. Could the File class interface be improved?

No question about it.  But I can't make it better _and_ keep the patch 
smallish.  The current patch was meant to be fairly modest in size and scope, 
so it does not attempt to "finish" the work.  Given the constraints, one can 
not reasonably expect the new class to be perfect in the first pass.

> 3. Should there be an EmbeddedNetworkFile?

Absolutely.  In fact, I almost think it should be called NetworkFile, because 
it properly hides the storage mechanism.  I believe it is a poor design that 
exposes the fact that we store our networks with XML; this new class hides 
the implmentation details, and I even included my patch for NetworkEditor to 
re-use it to load/save its networks.  

There are other details that could be hidden in this class, like whether or 
not an embededed network should be saved externally (to a disjoint file) or 
as part of its containing network.  To a certain extent, I think this begins 
to suggest that EmbeddedNetworkFile might want to inherit from NetworkFile, 
providing such extensions beyond simple file loading/saving.  That can be 
saved for a later patch, though.

Further, the APIs that were questioned by Pau are used in the improved 
Qt4Configurator class to display the proper information in the configuration 
file dialogs for AudioFile{Source,Target} instances.  So to be clear, these 
methods were added because they abstract away the interface for loading and 
saving files with "restricted types".  They allow consumers to meaningfully 
prompt the user for the proper type(s) of files, provided by a set of file 
extensions, and are actively being used for that purpose.

All said, please reconsider applying the existing patch.  I do not expect it 
to be the end to this line of development, but I believe it should give a 
fine start.  Right now, the AudioFile class may be the only consumer, but 
these changes lay the foundation for further and immediate re-use. There are 
a few areas that I have already realized can use some additional improvement, 
but they will be much easier to work on if applied later and separately.  

Cheers,

Zach