Re: [Libreoffice] [Libreoffice-commits] .: stoc/source

[Stephan Bergmann]

On 10/04/2011 07:11 PM, Kevin Hunter wrote:

At 9:33am -0400 Mon, 03 Oct 2011, Tor Lillqvist wrote:

stoc/source/inspect/introspection.cxx | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)

New commits:
commit 9e6d06a871b366cc72f9a23ab45080b66a47f144


Making my way through a backlog of commits ... it seems to me that it
doesn't matter why the for-loop was used in the first place, because now
it's not clean code, and the function could be made that much better.

I don't know much about the Reference or XIdlClass data types, but this
patch at least compiles on my machine, and I claim removes ambiguity
while remaining true to the came-before logic. It also removes the
overhead of recursion, as I don't think this algorithm needs it.


Hi Kevin,

Thanks a lot for the patch.  I think the real intent always was to 
actually look through all the returned getSuperclasses(), and the error 
that superclasses past the first one are effectively ignored has never 
been noticed.  (getSuperclasses() returning a sequence of length greater 
than one only happens for multiple-inheritance interface types, which 
are relatively rare.  And the isDerivedFrom in question is probably also 
not called that much.)


I will look into this tomorrow.  (I had this oddity on my list for a 
while now, anyway.)


-Stephan
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice

Re: [Libreoffice] [Libreoffice-commits] .: stoc/source

[Kevin Hunter]

At 4:15pm -0400 Tue, 04 Oct 2011, Stephan Bergmann wrote:

Thanks a lot for the patch. I think the real intent always was to
actually look through all the returned getSuperclasses(), and the
error that superclasses past the first one are effectively ignored
has never been noticed.


Excellent.  Was wondering, but don't yet know LO well enough to make 
such declarations.  Well, modulo any errors on my part, the logic I sent 
in patch 1 should be the same as what was originally there, but I hope 
easier to read/see for comparison/fixing.


Here is a second patch that compiles, /should/ respond to what you just 
confirmed was the original intent, but is untested.  (It was a random 
drive by patching.)  Specifically, I suppose it's obvious that this now 
changes the semantics of it actually used to do.  If you know how to 
test it ...


Cheers,

Kevin
From 0543ea7fad724856ee1ed837129808815eb44480 Mon Sep 17 00:00:00 2001
From: Kevin Hunter hunt...@earlham.edu
Date: Tue, 4 Oct 2011 12:58:10 -0400
Subject: [PATCH] Fix logic of isDerivedFrom function

From an email conversation with Stephen Bergmann

I think the real intent always was to actually look through all the
returned getSuperclasses(), and the error that superclasses past the
first one are effectively ignored has never been noticed.
---
 stoc/source/inspect/introspection.cxx |   25 +++--
 1 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/stoc/source/inspect/introspection.cxx b/stoc/source/inspect/introspection.cxx
index 36f1acc..7d62578 100644
--- a/stoc/source/inspect/introspection.cxx
+++ b/stoc/source/inspect/introspection.cxx
@@ -110,29 +110,18 @@ sal_Bool isDerivedFrom( ReferenceXIdlClass xToTestClass, ReferenceXIdlClass
 {
 Sequence ReferenceXIdlClass  aClassesSeq = xToTestClass-getSuperclasses();
 const ReferenceXIdlClass* pClassesArray = aClassesSeq.getConstArray();
+
 sal_Int32 nSuperClassCount = aClassesSeq.getLength();
-sal_Int32 i;
-for( i = 0 ;
- i  nSuperClassCount ;
- /* No increment expression needed as the body always
-  * returns, and in fact MSVC warns about unreachable code if
-  * we include one. On the other hand, what's the point in
-  * using a for loop here then if all we ever will look at is
-  * pClassesArray[0] ?
-  */ )
+for ( sal_Int32 i = 0; i  nSuperClassCount; ++i )
 {
 const ReferenceXIdlClass rxClass = pClassesArray[i];
-if( xDerivedFromClass-equals( rxClass ) )
-{
-// Treffer
+
+if ( xDerivedFromClass-equals( rxClass ) ||
+ isDerivedFrom( rxClass, xDerivedFromClass )
+   )
 return sal_True;
-}
-else
-{
-// Rekursiv weitersuchen
-return isDerivedFrom( rxClass, xDerivedFromClass );
-}
 }
+
 return sal_False;
 }
 
-- 
1.7.1

___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice