[Box Backup-dev] COMMIT r348 - in box/chris/bb-save-state: . bin/bbackupd lib/common lib/server

[Ben Summers]

On 29 Jan 2006, at 22:33, Chris Wilson wrote:

> Hi Ben,
>
> On Sun, 29 Jan 2006, Ben Summers wrote:
>
>> I would have removed the operator overloading from Archive, and  
>> personally would have just required that classes derived from  
>> Archive only provide Read(void*,int length) and Write(void*,int  
>> length) functions, rather than doing absolutely everything, and  
>> made base class Get and Put inline functions which call those.
>
> I considered doing that, but I didn't want to introduce too many  
> changes at once.

Go wild! It's not as if it's in trunk.

>
> As you spotted, I didn't remove all the operator overloading, only  
> in BackupDaemon.cpp.
>
>> bin/bbackupd/BackupClientDirectoryRecord.h
>> bin/bbackupd/BackupDaemon.h
>> lib/common/ExcludeList.h
>> 	-- #include "Archive.h" -> class Archive;  ?
>
> Sorry, I don't understand what's wrong.

Well, if you only refer to a class by it's name as pointers or  
references, such as when you're defining it as an argument to a  
function in a .h file, you don't need to include the file. You've  
just got to tell the compiler that Archive is actually a class.

If you include files unnecessarily in header files, it means that  
changes cause far too many files to be recompiled as of course  
anything which includes either of those files is now dependent on  
Archive.h. If you're not careful, you find that modifying almost any  
file means that everything gets rebuilt.

>
>> As I understand it, I just need to adjust the infrastructure  
>> building to run ./bootstrap, and then include all the new files  
>> from SVN in the root, along with the generated configure?
>
> Seems reasonable to me.

Good.

Onto the code... I wonder if it would be better to just delete the  
Archive class, and rename IOStreamArchive to Archive. I don't quite  
see what this gives us, except added complexity and the overhead of  
virtual function calls, given that you can get all the flexibility  
you need by simply choosing your IOStream appropriately. Then Archive  
becomes a very simple class, calls to which compile down to calls to  
the underlying IOStream object.

Also, in my experience, defining virtual functions inline in header  
files causes problems with compilers. I'm sure MSVC has been fixed  
now, but still... :-)

OTOH, perhaps we should just merge it and get on with things?

Ben