Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compare multiple objects in Variable View #581

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

SougandhS
Copy link
Contributor

@SougandhS SougandhS commented Dec 19, 2024

This commit provides a new feature to compare elements of commonly used objects of same types like Map, HashMap, LinkedHashMap, ConcurrentHashMap, Hashtable , IdentityHashMap, WeakHashMap, TreeMap, List, ArrayList, Stack, Vector, LinkedList, HashSet, LinkedHashSet, TreeSet, CopyOnWriteArraySet, Arrays, Wrappers and custom objects from Variables view and display differences of both either in diff view or in dialog box.

Fixes eclipse-platform/eclipse.platform#531

How to test

->Select atleast 2 objects of same type (List, Map, Set etc) from Variables view
->Choose "Compare" from menu

image Result

image


image


image


Screenshot 2024-12-31 at 9 53 23 AM


Screenshot 2024-12-31 at 9 52 35 AM


Screenshot 2024-12-31 at 9 49 32 AM


Screenshot 2024-12-31 at 9 46 13 AM


Screenshot 2024-12-31 at 9 40 28 AM


image


Screenshot 2024-12-31 at 12 29 10 PM


Screenshot 2025-01-03 at 4 25 50 PM


Screenshot 2024-12-31 at 3 56 05 PM


Screenshot 2024-12-31 at 3 54 10 PM


Screenshot 2024-12-31 at 9 54 44 AM


Screenshot 2024-12-31 at 9 46 13 AM

Author checklist

@SougandhS SougandhS force-pushed the Compare_2_Objects branch 2 times, most recently from 3db6961 to 368b954 Compare December 19, 2024 09:15
@SougandhS
Copy link
Contributor Author

Hi @jukzi , could you please check this PR when you have time.

@jukzi
Copy link
Contributor

jukzi commented Dec 19, 2024

have not looked at the sourcecode, only on the screenshots yet:
Sounds like a usefull feature
However It is not intuitive to understand to the user if the comparison compares equality (.equals()) or identity (==) and how it is done for nested collections/arrays (deep equals?)

@SougandhS
Copy link
Contributor Author

SougandhS commented Dec 19, 2024

have not looked at the sourcecode, only on the screenshots yet: Sounds like a usefull feature However It is not intuitive to understand to the user if the comparison compares equality (.equals()) or identity (==) and how it is done for nested collections/arrays (deep equals?)

it actually extracts contents in list,map,sets and objects and stores it as a string data then compares those data with equals()
suppose we have lists of
List l1 = new ArrayList<>(Arrays.asList("apple", "banana"));
List l2 = new ArrayList<>(Arrays.asList("apple", "banana"));
and if we do l1.equals(l2) it results true
but if we do l1.equals(l2) for list like this (having same contents but in different order) ->
List l1 = new ArrayList<>(Arrays.asList("apple", "banana"));
List l2 = new ArrayList<>(Arrays.asList("banana", "apple"));
it results false

but if we now select these objects and compare from menu we get "elements contain same data"

and it wont go nested extraction for that it simply compares with the object name and id (check reference / identity)

if (referenceTypeObject1.equals(referenceTypeObject2)) {

String selectionReference = selectedObject1.getReferenceTypeName();
if (selectionReference.equals("java.lang.String")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you use interface CharSequence instead of String implementation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from getReferenceTypeName() and javaType() only "java.lang.String" is getting returned.

Copy link
Contributor

@jukzi jukzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All messages displayed to user (execept exceptions) should be multi language aware.

@jukzi
Copy link
Contributor

jukzi commented Dec 19, 2024

It only compares exactly 2 values? Wouldn't it make sense to compare also n values?

@jukzi jukzi added enhancement New feature or request noteworthy Noteworthy feature labels Dec 19, 2024
@jukzi
Copy link
Contributor

jukzi commented Dec 19, 2024

i don't get why #203 (comment) says it should be implemented in platform but this is a jdt.debug only PR

@jukzi
Copy link
Contributor

jukzi commented Dec 19, 2024

it actually extracts contents in list,map,sets

I understand what it does when i look at the code but i wonder if/how that could be expressed to the user.
And what can be expected for comparing two java.util.SortedSet that have same items but different order? Are they equal? I would hope it's not only true/false but saying they have same elements but different order. And such answer could also be a valid statement of anything that does not implements Set but also for an Array for example. While a object that only implements Set without SequencedCollection can not be compared for order. However SequencedCollection is only JDK 21, so what will be the heuristic for jdk<21?

@SougandhS
Copy link
Contributor Author

SougandhS commented Dec 19, 2024

All messages displayed to user (execept exceptions) should be multi language aware.

Okay

It only compares exactly 2 values? Wouldn't it make sense to compare also n values?

As of now it compares only 2 according to the #203 , for n comparisons it will be too complex

@iloveeclipse
Copy link
Member

If that is supposed to be implemented, it should use diff viewer support for complex types, not dialogs.

@jukzi
Copy link
Contributor

jukzi commented Dec 19, 2024

for n comparisons it will be too complex

Comparing n elements is only a for-loop of comparing n-1 times 2 values each.

@SougandhS
Copy link
Contributor Author

I understand what it does when i look at the code but i wonder if/how that could be expressed to the user. And what can be expected for comparing two java.util.SortedSet that have same items but different order? Are they equal? I would hope it's not only true/false but saying they have same elements but different order. And such answer could also be a valid statement

Yes. Totally agree. I will provide additional msgs like you suggested.

While a object that only implements Set without SequencedCollection can not be compared for order. However SequencedCollection is only JDK 21, so what will be the heuristic for jdk<21?

Actually in current implementation there is a reference check provided prior comparison, so SequencedCollection will only be able to compare other SequencedCollection

@SougandhS
Copy link
Contributor Author

If that is supposed to be implemented, it should use diff viewer support for complex types, not dialogs.

Okay

@SougandhS SougandhS changed the title Compare two objects in Variable View #531 Compare multiple objects in Variable View #531 Dec 31, 2024
@SougandhS
Copy link
Contributor Author

SougandhS commented Dec 31, 2024

I made few more changes and included a custom diff view for custom objects and complex objects as per @iloveeclipse suggested. now it supports n-comparisons and List,Map,Set can be campared to any of its implementations . If selected objects are only 2 then dialog box is only used if its more than 2 then diff is used. Also for Sets comparison I have eliminated its order checking to eliminate confusion.

image


image


image




image


image





image




image


for custom objects fields are compared with each other

image image




for nested objects their identity/reference is checked
image
image




image




image


image





image





image




image





2 new exceptions with diff string msg
image
2 new exceptions with same string msg
image

jukzi
jukzi previously requested changes Jan 3, 2025
Copy link
Contributor

@jukzi jukzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that code became really long. Unfortunatly it is hard to read when the declarations are not at the initializing code.

@SougandhS
Copy link
Contributor Author

Wow, that code became really long. Unfortunatly it is hard to read when the declarations are not at the initializing code.

Will clean it further more

@SougandhS SougandhS requested a review from jukzi January 6, 2025 03:50
@SougandhS SougandhS requested a review from jukzi January 6, 2025 10:09
@SougandhS SougandhS force-pushed the Compare_2_Objects branch 8 times, most recently from 2b8a74c to b2c16b6 Compare January 9, 2025 04:56
Copy link
Contributor

@jukzi jukzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks much better now, i don't know when i would have time to test it. maybe meanwhile some one else wants to take a look? @iloveeclipse ?

@jukzi jukzi dismissed their stale review January 9, 2025 07:24

review comments applied

This commit provides a new feature to compare elements of commonly used
objects of similar types like Map, HashMap, LinkedHashMap,
ConcurrentHashMap, Hashtable , IdentityHashMap, WeakHashMap, TreeMap,
List, ArrayList, Stack, Vector, LinkedList, HashSet, LinkedHashSet,
TreeSet, CopyOnWriteArraySet, Arrays, Wrappers and custom objects from
Variables view and display differences of both in either diff viewer or
dialog box.

Fixes eclipse-platform/eclipse.platform#531
@jukzi
Copy link
Contributor

jukzi commented Jan 10, 2025

i tested with

public class A {
	public static void main(String[] args) {	
		Set<String> s1 = java.util.concurrent.ConcurrentHashMap.newKeySet();
		s1.add("1");
		Set<String> s2 = Set.of("2");
		Set<String> s11 = Set.of("1");
		List<String> l1 = List.of("1");
		System.out.println(s1.equals(s11)); // true
		System.out.println(s1.equals(s2)); //false
		System.out.println(s1.equals(l1)); //false
	}
}

This message seems to be inconsitent:
image
either name both objects i.e. "s1 equals s11" or none, i.e. "Objects are equal"

For the record, the other messages shown are:
image
image

I dislike the "error" icon. its not an error when objects are of different type. its just a result of comparison:
image

@jukzi
Copy link
Contributor

jukzi commented Jan 10, 2025

I also tried to compare 3 objects.

		Set<String> s2 = Set.of("2","3");
		Set<String> s11 = Set.of("1","3");
		Set<String> s3 = Set.of("4","3");

Results in:
image
looks ugly: "SL No." is unclear. dark red backgroundcolor makes a bad contrast,. The "Missing Elements" column is hard to understand. The simple Listing in the Variables View is much better to understand:
image

@jukzi
Copy link
Contributor

jukzi commented Jan 10, 2025

For Lists:

		List<String> l1 = new ArrayList<>();
		l1.add("1");
		l1.add("3");
		List<String> l2 = List.of("3","1");

image
instead of "Object" better say "Collections contain same elements, but in different order" where "Collection" should be the Interface that you actually used for comparison (i did not check which Interface the implementation uses)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request noteworthy Noteworthy feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compare two objects in Variable View
3 participants