Dear Oleg, thanks for your comments.
> generates the type declaration. I've add commented type declaration > here only to increase the readability of the code. Do you think that > I should get rid of such comments? yes. > >If the whole point is to get rid of these positional list of doubles, why > >generate such a function? Can't we just generate directly the functions that > >use it, i.e., compCVfromStats and printStats via template haskell? > > > > Yes, it's possible to do it but the code will become a bit more > complex. I'm still strongly in favour of it. In my opinion, if we refactor some piece of code, it should be done in such a way that the structure becomes maintainable. And maintainability mainly depends on which modules are there, which functions they export, and the signatures of the functions inside the module. The latter is actually a lot more important as it might seem: module boundaries are a weak encapsulation, as patches extending the list of exported functions unfortunately usually don't get reviewed as strictly as they should. Also, people will have to do changes inside the module. So the structure of the official (i.e., generated) code is quite important. And there, I much prefer functions only acting on the clear meaningful data types, i.e., - nodes, - a compound type containing all the data of the node relevant for the statistics, all clearly named, - a compound type containing all the accumulated statistics, all clearly named, and - the cluster metric value (a number). And if we can hide all intermediate values inside the definition of functions, I have no problems with quite some more template haskell code. In my opinion, having that list of doubles available (even if hidden inside the module) is getting us on that slippery slope to unmaintainable code again. Also, from a perspective of type safety: with a list of doubles instead of a proper record type, you're losing information about the length of the list (let alone the positional meaning of the numbers). Note that the compiler will not detect it, if you do some manipulations of the list inbetween. > >>+-- | Compute the statistics of a cluster given the nodes > >>+compClusterStatistics :: [Node.Node] -> ClusterStatistics > >>+compClusterStatistics nl = > >>+ compClusterStatisticsHelper $ partition Node.offline nl > > > >Why genrate a helper with template haskell and not directly > >compClusterStatistics? > >I don't see where the helper is used anywhere else or useful in itself. > > > > Again, to simplify the template haskell code. simple partition > Node.Offline nl requests about 3-4 lines of template haskell code. Also, here again, I would happily accept a dozen lines more if by that the functions visible inside the module have a more clear meaning and a nicer signature. Thanks, Klaus -- Klaus Aehlig Google Germany GmbH, Dienerstr. 12, 80331 Muenchen Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschaeftsfuehrer: Graham Law, Christine Elizabeth Flores
