622993 - Bugzilla::Util::diff_arrays could diff also object arrays based on an object attribute

Status: RESOLVED DUPLICATE of bug 620827

(Bugzilla :: Bugzilla-General, enhancement)

Description

[Tiago Mello [:timello]]

Attachment: v1

That would avoid dealing with map/grep all the time for comparing object arrays.

Comment 1

[Frédéric Buclin]

Comment on attachment 501152 [details] [diff] [review]
v1

>=== modified file 'Bugzilla/Util.pm'

>             next if ($newv eq '');

You are comparing an object with a string?


>+            if (defined $attrib and $oldv->$attrib eq $newv->$attrib) {
>+                $newv = $oldv = undef;
>+            }
>+            elsif ($oldv eq $newv) {
>                 $newv = $oldv = '';
>             }

The first "if" statement is incorrect. If $attrib is defined, but $oldv->$attrib is not equal to $newv->$attrib, you would jump to the "elsif" block.

Comment 2

[Tiago Mello [:timello]]

Attachment: v2

Comment 3

[Max Kanat-Alexander]

This will also need to include some code that actually uses this form of diff_arrays, in order for us to check it in.

Also, I'm pretty sure that we'd always be comparing by id--perhaps we should just start with a diff_arrays that always compares objects by id?

Comment 4

[Frédéric Buclin]

Comment on attachment 501163 [details] [diff] [review]
v2

Per mkanat's comment, nothing uses the new feature yet. I'm not sure about ->id being the most used method for comparison, though.

Comment 5

[Tiago Mello [:timello]]

(In reply to comment #4)
> Comment on attachment 501163 [details] [diff] [review]
> v2
> 
> Per mkanat's comment, nothing uses the new feature yet. I'm not sure about ->id
> being the most used method for comparison, though.

Ok, we are going to 'use' this in the remove see also patch, so this will be implemented there.