View Issue Details

IDProjectCategoryView StatusLast Update
0003650JEDI VCL00 JVCL Componentspublic2006-05-03 05:31
Reporterivan_raAssigned Toivan_ra 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product VersionDaily / GIT 
Target VersionFixed in Version3.30 
Summary0003650: Strange code in IvInterpreter
Descriptionthere is strange code introduced in rev.10397. This code "fixes" 0002676 and located in TJvInterpreterIdentifierList.Find:

  while (I <= Count - 1) and (AnsiStrIComp(PChar(TJvInterpreterIdentifier(List[I]).Identifier), PChar(Identifier)) = 0) do
   if SameText(TJvInterpreterMethod(List[I]).FClassType.ClassName, ClassName) then begin Index := I; exit; end else inc(I);

and some code in TJvInterpreterAdapter.GetValue.GetMethod
Additional InformationNew code in TJvInterpreterIdentifierList.Find finds method for class naming ClassName.
1) in common, this code is unsafe because TJvInterpreterIdentifier<>TJvInterpreterMethod
2) this code is unnecessary, because all methods sorted with inheritance (look at TJvInterpreterMethodList.Sort), so all methods saved in list by inheritance:

"Method"
"Method" from ancestor
"Method" from ancestors ancestor
...

So, original code does everything - it finds first ancestor's method, in particular, exactly objects class method first
TagsNo tags attached.

Relationships

related to 0003667 resolvedivan_ra JvInterpreter: non-standard var params does not works 

Activities

ivan_ra

2006-04-20 10:31

developer   ~0009156

Last edited: 2006-04-20 10:31

Issue 0002676 was posted for beta version JVCL
It seems now this fix is unnecessary, and more, changes interface without need, and demonstrate bad style of programming

obones

2006-04-20 11:18

administrator   ~0009158

But I did see the bug happening. So there was an issue. The code might be strange, that I agree. But it does work. If you have "cleaner" code, then please provide it.

ivan_ra

2006-04-21 03:56

developer   ~0009166

Could you post example? I think error was in user application, not in Interpreter source code. Here is my example:
class TZtringList inherits from TStringList and sorts strings in descending order. It has interpreter adapter and works fine with "old good code" exactly as I wrote

2006-04-21 03:58

 

JvInterpreter.zip (51,906 bytes)

ivan_ra

2006-04-21 09:28

developer   ~0009168

I want to say what fix from 0002676 is harmless, but:
1) it does nothing new - all introduced logic is part of already implemented logic
2) it's changing interface without need - it's BAD

And this is "downgrading" patch for issue

2006-04-21 09:29

 

JvInterpreter.pas.svn.patch (1,983 bytes)
Index: JvInterpreter.pas
===================================================================
--- JvInterpreter.pas	(revision 10546)
+++ JvInterpreter.pas	(working copy)
@@ -384,7 +384,7 @@
     FDuplicates: TDuplicates;
   public
     function IndexOf(const UnitName, Identifier: string): TJvInterpreterIdentifier;
-    function Find(const Identifier: string; var Index: Integer; const ClassName: string = ''): Boolean;
+    function Find(const Identifier: string; var Index: Integer): Boolean;
     procedure Sort(Compare: TListSortCompare = nil); virtual;
     property Duplicates: TDuplicates read FDuplicates write FDuplicates;
   end;
@@ -2892,7 +2892,7 @@
 //=== { TJvInterpreterIdentifierList } =======================================
 
 function TJvInterpreterIdentifierList.Find(const Identifier: string;
-  var Index: Integer; const ClassName: string = ''): Boolean;
+  var Index: Integer): Boolean;
 var
   L, H, I, C: Integer;
 begin
@@ -2917,13 +2917,6 @@
     end;
   end;
   Index := L;
-
-  // Mantis 2676: Looking for our specific class, if applicable
-  if not Result or (ClassName = '') then
-    exit;
-  I := Index;
-  while (I <= Count - 1) and (AnsiStrIComp(PChar(TJvInterpreterIdentifier(List[I]).Identifier), PChar(Identifier)) = 0) do
-   if SameText(TJvInterpreterMethod(List[I]).FClassType.ClassName, ClassName) then begin Index := I; exit; end else inc(I);
 end;
 
 procedure TJvInterpreterIdentifierList.Sort(Compare: TListSortCompare = nil);
@@ -3794,13 +3787,7 @@
     if Result then
       Exit;
 
-    // Mantis 2676: Looking for the actual class name if appropriate
-    if Args.ObjTyp = varObject then
-      IdentifierFound := FGetList.Find(Identifier, i, Args.Obj.ClassName)
-    else
-      IdentifierFound := FGetList.Find(Identifier, i);
-
-    if IdentifierFound then
+    if FGetList.Find(Identifier, i) then
       for I := I to FGetList.Count - 1 do
       begin
         JvInterpreterMethod := TJvInterpreterMethod(FGetList[I]);

obones

2006-04-26 00:52

administrator   ~0009187

Ok, I must have mixed up two issues, I thought there was an example with 2676.
What is in the JvInterpreter.zip file you attached here?

ivan_ra

2006-04-26 05:16

developer   ~0009190

This is example with descendant class to check up 0002676:

TZtringList = class (TStringList)
public
  procedure Sort; // not override
end;

This class has adapter and works in interpreter as in compiled code

1) after call of JvInterpreterMethodList.Sort methods sorted in this order:

TZtringList.Sort
TStringList.Sort

2) so, original method TJvInterpreterIdentifierList.Find always finds TZtringList.Sort before TStringList.Sort, and TJvInterpreterAdapter.GetValue.GetMethod exactly finds TZtringList.Sort for TZtringList and TStringList.Sort for TStringList

Issue History

Date Modified Username Field Change
2006-04-20 06:03 ivan_ra New Issue
2006-04-20 10:31 ivan_ra Note Added: 0009156
2006-04-20 10:31 ivan_ra Note Edited: 0009156
2006-04-20 11:18 obones Note Added: 0009158
2006-04-20 11:18 obones Status new => feedback
2006-04-21 03:56 ivan_ra Note Added: 0009166
2006-04-21 03:58 ivan_ra File Added: JvInterpreter.zip
2006-04-21 09:28 ivan_ra Note Added: 0009168
2006-04-21 09:29 ivan_ra File Added: JvInterpreter.pas.svn.patch
2006-04-26 00:52 obones Note Added: 0009187
2006-04-26 05:16 ivan_ra Note Added: 0009190
2006-05-03 05:30 ivan_ra Relationship added related to 0003667
2006-05-03 05:31 ivan_ra Status feedback => resolved
2006-05-03 05:31 ivan_ra Resolution open => fixed
2006-05-03 05:31 ivan_ra Assigned To => ivan_ra