[ag-automation] Questions/Annotations about implementation of fddi

[Thomas Rothfuss]

> 
>>1. Code documentation. There are tools like doxygen which generates
>>documentation from the source code. Therefore the functions and
>>structures have to be commented.  Are there intentions to use such a
>>tool?
> 
> 
> Yes, we'll use doxygen for API documentation. But I think at this stage
> it doesn't make any sense to start writing documentation, because the
> whole APIs are under heavy progress. Once they have stabilized, you're
> welcome to help in this area.
> 

Fine (since I know doxygen ;-)).

> 
>>2. Tests. There are some tests in the "tests" folder. They exit with 0
>>or in case of an error with another value.  There are test frameworks
>>like CppUnit for automatic testing.  Does it make sense to use such a
>>framework?
> 
> 
> Well, tests/ is the autotool method of doing tests. Automake generates
> makefiles which let you do
> 
> 	make check
> 
> and all the tests are running; the usual release procedure is to perform
> 
> 	make distcheck
> 
> which does the following:
> 
> - build the distribution tarball(s)
> - extract it again into a new directory
> - make this source directory read only
> - create a separate build directory
> - perform an out-of-tree build
> - run the testsuite in there
> 
> This way you can easily find out if you've forgotten to add something to
> the makefiles which should be part of the distribution.
> 
> Are there any features of CppUnit you are missing here?
> 
> 

If the testsuite can identify the location of the errors it should be
ok for me. In CppUnit you can use test runners like a GUI test
runner. I don't use this feature the tests have to run automatically.


>>3. Interface vs. implementation. In fddi_device.h there is following
>>    struct:
>>
>>        struct fddi_device {
>>
>>        	/* public */
>>        	fddi_id_t id;
>>        	fddi_state_enum_t state;
>>
>>        	/* private */
>>        	struct fddi_device *next;
>>        	struct fddi_device *previous;
>>
>>        	fddi_param_t *param_list;
>>
>>        	void *priv;
>>
>>        };
>>
>>    fddi_types.h:
>>
>>        typedef struct fddi_device fddi_device_t;
>>
>>
>>    I assume that "public" means the following members are used for the
>>    interface. "private" is part of the implementation.
> 
> 
> That's the idea.
> 
> 
>>    Since this should be a C interface not a C++ one fddi_device is not
>>    an abstract class to derive from. But it is possible to separate
>>    the interface from the implementation by using the pimpl-Pattern
>>    (see http://www.gotw.ca/gotw/024.htm). Using "void *priv;" is the
>>    a similar mechanism.
>>
>>    In this case the implementation fddi_device_impl needs a pointer
>>    to its interface class for iteration reason:
>>
>>        struct fddi_device_impl;  /* forward declaration or use void * */
>>
>>        struct fddi_device {
>>
>>        	fddi_id_t id;
>>        	fddi_state_enum_t state;
>>
>>        	struct fddi_device_impl * impl;
>>        	/* or maybe
>>         void * impl;
>>         */
>>
>>        };
>>
>>        struct fddi_device_impl {
>>
>>         fddi_device_t* i_fddi_device;
>>
>>        	struct fddi_device *next;
>>        	struct fddi_device *previous;
>>
>>        	fddi_param_t *param_list;
>>
>>        	void *priv;
>>
>>        };
> 
> 
> Sounds reasonable. I'll make some prototypes to check if it feels clean
> enough.
> 
> 

Good.

Regards,
Thomas