On 30/06/12 23:37, Philipp Riemer wrote: > => 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?
my experience tells me that SwClient is completely crazy and that changing it in any way probably breaks stuff. > => 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... hmm... it depends on whether it currently handles all possible values, or only some values. sadly i don't know what it does... > (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? yes, i tend to prefer "early returns", because it makes it immediately obvious that the method is not going to do anything any more in this case. ok, that is not so important in this 10 line function, but when the function is 500 lines long it helps. > => 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/>. hmmm why not... > => 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... yeah, looks like it wants to be an assert()... _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice
