Re: [Powertop] [RFC][PATCH 1/2] Adding report generator facility

[Sergey Senozhatsky <sergey.senozhatsky at gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1446 bytes --]

On (10/09/12 21:25), Igor Zhbanov wrote:
> >- I think null formatter could be a basic class with default
> >implementation (null) of virtual functions, rather than separate
> >implementation of basic formatter interface (abstract class).
> 
> One more note.
> 
> In my opinion making an empty implementation of the functions
> inside the interface class is not good.
> 
> The first reason, if now the programmer will forget to implement
> some method of the abstract interface, the program will not compile,
> instead of getting empty implementation and doing nothing.
>

yes, this is valid point, but at the same time also one of the reasons we have commit
review ;-)
 
> Second, I don't like to mix all types of report formatters because some
> of them would use string to hold the result, while another (probably XML)
> could use tree for DOM implementation). And not every formatter need
> escaping string function. So I have provides report_formatter_base
> as a common part for CSV and HTML report formatters but not for
> NULL formatter.
> 

in my example I don't think we mix anything. we just define a sub-set of common
funictions and implementations via base_impl classes. while this looks like a win
for current case (2 clases using string storage, so no code duplication), it may be
not as cool if every formatter class would use different data storage type: 
xml, json, proto buff, whatever.


	-ss