MappedField#equals should return false in comparison to a different field #1088

Closed
nafg opened this Issue Aug 8, 2011 · 4 comments

2 participants

@nafg

(Trying out
http://www.assembla.com/wiki/show/breakoutdocs/Email2TicketsGateway by
forwarding the thread.)

---------- Forwarded message ----------
From: David Pollak feeder.of.the.bears@gmail.com
Date: Sun, Aug 7, 2011 at 9:00 AM
Subject: Re: [Lift] Re: SortedMapperPaginatorSnippet initialSort
To: liftweb@googlegroups.com

On Sun, Aug 7, 2011 at 5:07 AM, Naftoli Gugenheim naftoligug@gmail.comwrote:

I could really just change it to eq instead of ==. You're point is true as
well, that I could do == on each's getClass.

Whoops... I forgot about eq... years of Scala has apparently blinded me to
stuff that's normal in Java... sigh. :-(

But, don't you agree that it would be correct for
MyMetaMapper.fieldA==MyMetaMapper.fieldB to return false?

Yeah... you're right (and thank you for being patient and walking me through
the implications.) Could you open a ticket? You're welcome to own the
ticket or assign it to me.

In scala a lot of things depend on a good equals implementation. For
instance, what if I want a Map[MappedField[,], X], or a
Set[MappedField[,]? Also, I think people in the FP world consider it bad
practice to use eq.

On Sat, Aug 6, 2011 at 4:21 AM, David Pollak <
feeder.of.the.bears@gmail.com> wrote:

AH... I understand now...

I do not know how to compare two JVM references to see if they point to
the same underlying object. It's possible in .Net, but not in Java.

In this particular instance, you might try comparing the class for each
field because each field has a discrete class (at least I think they should)
and the classes should be different between two different fields... does
this help?

On Sat, Aug 6, 2011 at 12:12 AM, Naftoli Gugenheim naftoligug@gmail.comwrote:

The real question is, given two MappedFields, what's the correct way to
test if they are they are the same field?
The context is that when you click a sort header of a paginator, it has
to check whether it's the currently selected header and if so toggle between
ascending and descending, or else default to ascending.

But being that the signature of equals is that it takes an AnyRef, I
think the principle of least surprise suggests that two different fields are
not equal, even if they have the same value (or, being on a MetaMapper
instance, having a meaningless value).

On Fri, Aug 5, 2011 at 5:31 AM, David Pollak <
feeder.of.the.bears@gmail.com> wrote:

On Fri, Aug 5, 2011 at 12:30 AM, Naftoli Gugenheim <
naftoligug@gmail.com> wrote:

DPP, is that desired behavior, that MyMetaMapper.fieldA ==
MyMetaMapper.fieldB?

I don't think it's valid to compare fields on the MetaMapper... they
should contain no valid values. Is that the question or did I miss
something?

On Thu, Aug 4, 2011 at 5:49 PM, Larry Morroni larry@morroni.comwrote:

They both evaluate to true on first page load and all successive page
loads.

On Jul 22, 6:19 pm, Naftoli Gugenheim naftoli...@gmail.com wrote:

Perhaps it's a bug. Can you tell me what the following evaluate to?
ClaimView.lastName == ClaimView.firstName
ClaimView.lastName == ClaimView.lastName

If the answers are not false and true respectively, then line 78 of
net/liftweb/mapper/view/Paginator.scala should be changed to use eq
(or the
equality implementation should change).

On Fri, Jul 22, 2011 at 2:34 PM, Larry Morroni la...@morroni.com
wrote:

seems that if I add
sort = (0,false)
then I get the list sorted by the first mapped item. It seems
like I
shouldn't need this since I already tell it what my initialSort is

On Jul 22, 12:09 pm, Larry Morroni la...@morroni.com wrote:

I am running Lift 2.4-M1 and I can't get the initialSort on
SortedMapperPaginatorSnippet to work. No matter what I set, it
sorts
by the column firstName. I can reproduce this on multiple
mappers of
mine. Does anyone see anything wrong with the syntax or could
this
possibly be a bug?

val paginator = new SortedMapperPaginatorSnippet(ClaimView,
ClaimView.lastName,
"updatedAt" -> ClaimView.updatedAt,
"firstName" -> ClaimView.firstName,
"lastName" -> ClaimView.lastName,
"claimNumber" -> ClaimView.claimNumber,
"streetAddress" -> ClaimView.streetAddress,
"dateOfLoss" -> ClaimView.dateOfLoss,
"status" -> ClaimView.status
)
{
...}

Thanks,
Larry

You received this message because you are subscribed to the Google
Groups
"Lift" group.
To post to this group, send email to liftweb@googlegroups.com.
To unsubscribe from this group, send email to
liftweb+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/liftweb?hl=en.

You received this message because you are subscribed to the Google
Groups "Lift" group.
To post to this group, send email to liftweb@googlegroups.com.
To unsubscribe from this group, send email to
liftweb+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/liftweb?hl=en.

--
You received this message because you are subscribed to the Google
Groups "Lift" group.
To post to this group, send email to liftweb@googlegroups.com.
To unsubscribe from this group, send email to
liftweb+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/liftweb?hl=en.

Lift, the simply functional web framework http://liftweb.net
Simply Lift http://simply.liftweb.net
Follow me: http://twitter.com/dpp
Blog: http://goodstuff.im

--
You received this message because you are subscribed to the Google
Groups "Lift" group.
To post to this group, send email to liftweb@googlegroups.com.
To unsubscribe from this group, send email to
liftweb+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/liftweb?hl=en.

--
You received this message because you are subscribed to the Google Groups
"Lift" group.
To post to this group, send email to liftweb@googlegroups.com.
To unsubscribe from this group, send email to
liftweb+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/liftweb?hl=en.

Lift, the simply functional web framework http://liftweb.net
Simply Lift http://simply.liftweb.net
Follow me: http://twitter.com/dpp
Blog: http://goodstuff.im

--
You received this message because you are subscribed to the Google Groups
"Lift" group.
To post to this group, send email to liftweb@googlegroups.com.
To unsubscribe from this group, send email to
liftweb+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/liftweb?hl=en.

--
You received this message because you are subscribed to the Google Groups
"Lift" group.
To post to this group, send email to liftweb@googlegroups.com.
To unsubscribe from this group, send email to
liftweb+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/liftweb?hl=en.

Lift, the simply functional web framework http://liftweb.net
Simply Lift http://simply.liftweb.net
Follow me: http://twitter.com/dpp
Blog: http://goodstuff.im

--
You received this message because you are subscribed to the Google Groups
"Lift" group.
To post to this group, send email to liftweb@googlegroups.com.
To unsubscribe from this group, send email to
liftweb+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/liftweb?hl=en.

@nafg

(In [[r:27e634cda314b54c6b4270b6615b87d8155d4867]]) Test #1088 -- MappedField#equals requires getClass to equal. Also, respect scala.Equals#canEqual

Branch: wip_1087_1088

@nafg

(In [[r:27e634cda314b54c6b4270b6615b87d8155d4867]]) Test #1088 -- MappedField#equals requires getClass to equal. Also, respect scala.Equals#canEqual

Branch: master

@nafg

(In [[r:0b5068b50c7b2eadccb8aad9be4bfa54c0f2ee1e]]) Merge branch 'wip_1087_1088'
Fixes #1087, #1088

  • wip_1087_1088: MappedField#canEqual: override is unnecessary Test #1087 -- Menu.param{s,}[T <: AnyRef] Test #1088 -- MappedField#equals requires getClass to equal. Also, respect scala.Equals#canEqual

Branch: master

@nafg nafg was assigned Mar 1, 2012
@nafg nafg removed their assignment Apr 7, 2014
@etorreborre etorreborre pushed a commit to etorreborre/framework that referenced this issue Dec 3, 2014
@nafg nafg Test #1088 -- MappedField#equals requires getClass to equal. Also, re…
…spect scala.Equals#canEqual
27e634c
@etorreborre etorreborre pushed a commit to etorreborre/framework that referenced this issue Dec 3, 2014
@nafg nafg Merge branch 'wip_1087_1088'
Fixes #1087, #1088

* wip_1087_1088:
  MappedField#canEqual: override is unnecessary
  Test #1087 -- Menu.param{s,}[T <: AnyRef]
  Test #1088 -- MappedField#equals requires getClass to equal. Also, respect scala.Equals#canEqual
0b5068b
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment