Hi, today I cleaned up the code in the directory "sw/source/core/attr/" and saw some things I am not sure about. Since I guess that it will not cost much time for an experienced LO dev to look at those "curious facts", I post them here on the list:
(1) Where should I put documentation? At the moment I just translate every (German) text I find in the source files into English. But I was wondering if this is useful... Where does Doxygen look for documentation? Header file declarations or directly at the function implementation? => sw/source/core/attr/calbck.cxx (2) The two functions "SwClient* SwClientIter::First" and "SwClient* SwClientIter::Last" seem to differ in their logic. Same seems to be true for "SwClientIter::Next" and "SwClientIter::Previous". Or is this marginal change in each case correct? Can please anyone more experienced than me take a quick look at those functions? => sw/source/core/attr/cellatr.cxx (3) The switch statement in "SwTblBoxFormula::ChangeState" has no default case. This might be helpful to capture bogus behaviour... (4) The function "SwTblBoxFormula::Calc" has only an "if" statement with no "else" case and also no other code outside. Is this intended? If so, wouldn't it be more readable if the logic is reversed, e.g. "if( rCalcPara.rCalc.IsCalcError() ) return; //explanation why", and everything else is written without being in an "if" block? => sw/source/core/attr/fmtwrapinfluenceonobjpos.cxx (5) The switch statement in "SwFmtWrapInfluenceOnObjPos::QueryValue" has only one case and thus can be replaced by an if/else; same for switch statement in "SwFmtWrapInfluenceOnObjPos::PutValue". See <https://gerrit.libreoffice.org/#/c/251/>. => sw/source/core/attr/format.cxx (7) Switch case 0 in "SwFmt::Modify" seems to be strange since it seems to be unexpected (from the comment) but returns directly without logging an error or the like... Hope you all enjoy your weekend! Cheers, Philipp _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice