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

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.