[Clam-devel] [PATCH] IMPORTANT change: making Processing::GetConfigure not mandatory

[David García Garzón]

1- I would add a 'typedef NullProcessingConfig Config' on the base Processing. 
Derived processing classes could still redefine such inner type to use other 
ProcessingConfig other than the null one.

2- Currently processings are unconfigured until you call Configure with the 
proper configuration object type (normally the constructor calls it with a 
default configuration object). On the implementation Pau sent, by default 
processing objects are configured without even having called Configure. I 
think this is not convenient. I think ConcreteProcessing implementers should 
call Configure(Config()) on the constructor or the processing should be 
unconfigured.

3- About the state enum, i would like to change not the name of the enums but 
improving the readibility of the code by adding some boolean method such:

bool IsConfigured() const {return mExecState!=Unconfigured; }
bool IsRunning() const {return mExecState==Running; }

Notice that a running processing is also a properly configured one.



On Dilluns 14 Maig 2007, Pau Arumi wrote:
>       Problem:
>
> Processing interface include the pure abstract method.
> virtual ProcessingConfig& GetConfig() const = 0;
>
> In many cases processings does not need a configuration object,
> however they are forced to implement GetConfig. Over the years
> we've used different solutions for this case:
> 1- create a custom empty config class, and have a processing
> attribute of this config class, which is returned in GetConfig()
> 2- implement GetConfig() with a throw 0; an assert or similar, so
> the compiler does not complain about the non-existing return
> statement.
> 3- (the way documented) in  the wiki: declare a static variable
> of NullProcessingConfig inside GetConfig() and return it:
> ProcessingConfig GetConfig() const
> {
> 	static NullProcessingConfig config;
> 	return config;
> }
>
>
> The second problem is related to the constructor. Processings
> writers have to write a default constructor that calls Configure(
> ...)
> passing the default config. Otherwise, the default
> constructed processing will assert on Do time.
>
> The solution at the wiki is:
> 	typedef CLAM::NullProcessingConfig Config;
> 	MyProcessing(const Config & config=Config())
> 	{
> 		Configure( config );
> 	}
>
>
> None of these solutions is very obvious and make writing simple
> processings quite difficult
>
>
>      Solution
>
>
> Implement virtual ProcessingConfig& GetConfigure() const at the
> base class, returning a static NullProcessingConfig.
>
> Change behaviour of Processing::Processing() so it leaves the
> processing in Configured state.
> So without the constructor and GetConfig methods a simple
> processing looks like this:
>
> class MyProcessing : public CLAM::Processing
> {
> 	CLAM::InPort<float> mIn;
> 	CLAM::OutPort<float> mOut;
> public:
> 	const char* GetClassName() const { return "MyProcessing"; 	}
>   	~MyProcessing() {}
>
> 	// Configuration to the minimal expression
> 	bool ConcreteConfigure(const CLAM::ProcessingConfig&) { return
> true; }
> 	bool Do()
> 	{
> 		Do(mIn.GetData(), mOut.GetData());
> 		// Tell the ports this is done
> 		mIn.Consume();
> 		mOut.Produce();
> 		return true;
> 	}
>
> 	bool Do(const float& in, float& out)
> 	{
> 		// Your implementation
> 	}
> }
>
>
> If nobody discusses the solution in 24 hours I'll commit the
> attached patch ;)
>
> Pau



-- 
David García Garzón
(Work) dgarcia at iua dot upf anotherdot es
http://www.iua.upf.edu/~dgarcia
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.clam-project.org/pipermail/clam-devel-clam-project.org/attachments/20070515/64cdf74d/attachment-0003.pgp>