[scribus-dev] 0012245: Grouped lines appear thicker

Craig Bradney cbradney at scribus.info
Tue Nov 17 21:21:39 UTC 2015


> 
> so, how can we then document what DrawObj_item() is doing?
> for sure by writing some doxygen doc for the method.
> but also by splitting the 90 lines long method in a few private function with telling names, names that automatically tell the reader what is going on.
> 
> 
> so, my question to the team is: would be it be ok to ask as. to create a new private function with the code around
> 
> p->scale(m_width / groupWidth, m_height / groupHeight);
> 
> so that it's clear what that line does?
> 
> in my eyes, in such a case, the programmer should not refactor the whole DrawObj_Item() function, just the lines that she fears are offending.
> the refactoring should be submitted as a separate patch.
> and only after having refactored the code (and in this way having shown that she understood what the specific line is doing) she should apply the change and submit a patch for it.
> 
> of course, the patch for refactoring the existing method, should be submitted even if the bug was not there :-)
> 
> and, of course, not every line should be in its own function! but in my experience 10 lines is somehow the ideal length or a function.
> 
> 
> and my proposal to HOST:
> 
> - when you touch a function, always try to write a doxygen doc for it and make a patch with it (if the team agrees that there is a need for it, of course... and the comment should describe what the function does and not be obvious: no /** @return returns x */ ABC::getX() {return x;}, please :-)
> 
> - when you see a function that does multiple things that are not summarized in the function name, split the function in private function that are specific to the task (always if the team agrees)
> 

if you want to make functions all 10 lines long, there would be hundreds of thousands of functions. Good luck with the function calling slowdown and the horrible mess you introduce. 

Refactoring is more useful when you find common code that has been repeated in some way, especially across classes. Refactoring just to break up functions will often just make debugging and code paths very complex. 

Craig


More information about the scribus-dev mailing list