To comment on the following update, log in, then open the issue:
http://www.openoffice.org/issues/show_bug.cgi?id=46681
------- Additional comments from [EMAIL PROTECTED] Tue Jun 3 21:58:39 +0000
2008 -------
Hi Yue,
The approach looks viable in general. Some comments:
In real life (see remarks further down about copying methods), the
implementation of SetNewResMat() in the final version should not be in
the header file, but in a newly to be created jumpmatrix.cxx instead.
Header files should contain only relatively short inline methods. The
nested for() loops wouldn't necessarily be inlined by most compilers
anyway. The method is also quite rarely called, so we wouldn't gain
anything but increase compile time instead each time the header file is
parsed.
Instead of
ScMatrix* pNewMat = new ScMatrix(...
[...]
ScMatrix* p = pMat;
pMat = pNewMat;
p->Delete();
use
ScMatrixRef xNewMat = new ScMatrix(...
[...]
pMat = xNewMat;
Reason is that ScMatrixRef does refcount the ScMatrix, the pMat member
variable is also a ScMatrixRef, and by directly calling Delete() on the
pointer you interfere with the refcounting mechanism and may actually
doubly delete the matrix that was already released during the assignment
to pMat.
The copying code
for (SCSIZE nC = 0; nC < nResMatCols; nC++)
for (SCSIZE nR = 0; nR < nResMatRows; nR++)
is error prone; in case we added more type functionality to ScMatrix it
would had to be adapted and might easily get overlooked. It already
omits the IsEmptyPath() case ;-)
Better create a ScMatrix member method that copies the desired portion
similar to ScMatrix::MatCopy(), which would be faster as well. If you
also add a method ScMatrix::CloneAndExtend(nNewCols,nNewRows) or so,
similar to ScMatrix::Clone(), most code of ScJumpMatrix::SetNewResMat()
could be moved into these two methods, and SetNewResMat() could remain
inline in the header file.
Btw, last nitpick for now is a spelling error, Ajust should be written
with a 'd': Adjust ;)
Hope that helps.
Eike
---------------------------------------------------------------------
Please do not reply to this automatically generated notification from
Issue Tracker. Please log onto the website and enter your comments.
http://qa.openoffice.org/issue_handling/project_issues.html#notification
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
