You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by "Michael Papp (JIRA)" <ji...@apache.org> on 2008/03/12 21:28:47 UTC
[jira] Created: (SHINDIG-124) Implementation POJO for
opensocial.Enum
Implementation POJO for opensocial.Enum
---------------------------------------
Key: SHINDIG-124
URL: https://issues.apache.org/jira/browse/SHINDIG-124
Project: Shindig
Issue Type: New Feature
Components: Gadgets Server - Java
Reporter: Michael Papp
Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
Re: [jira] Commented: (SHINDIG-124) Implementation POJO for
opensocial.Enum.xx and opensocial.Message
Posted by Cassie <do...@google.com>.
Taking off the bug because this has turned into a longer thing..
On Tue, Mar 18, 2008 at 7:03 PM, Michael Papp (JIRA) <ji...@apache.org>
wrote:
>
> [
> https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12579963#action_12579963]
>
> Michael Papp commented on SHINDIG-124:
> --------------------------------------
>
> Hi Cassie - thanks again for your assistance with this code.
>
> 1. Will remove author tag
>
> 2. Here is my reasoning for the implementation:
> a. Rules for Enum:
> displayValue can never be null.
> Key may be null.
> if displayValue is one of the pre-defined constants, then
> displayValue == Key.toString().
I think this assumption is where things break. We shouldn't assume it will
be key.toString() it should be allowed to be an arbitrary string as well. If
we want to include a helper constructor that just takes in the enum only and
uses enum.toString for the displayValue then that is fine. it only needs to
be a helper though.
>
> if displayValue is not one of the pre-defined constants,
> then Key = null; displayValue = arbitrary string.
> b. Enum(Key key, String displayValue) is an unnecessary constructor, as
> either the Key is equivalent to the Value (String), or the Key != Value and
> the Key is null. Therefore, only the displayValue matters - not the Key.
> Still, for conformity, I could implement this constructor and throw away
> the Key.
if we go this route, you should really throw away the displayValue... not
the key. then you wouldn't need all of the lookup logic. however, as i
mentioned above, i think both should be preserved in this constructor. so
perhaps this:
public Enum(String displayValue) {
this(null, displayValue)
}
public Enum(T key) {
this(key, key.toString())
}
public Enum(T key, String displayValue) {
// set both
}
>
> c. The reverse look up on the enum Key properties enforces this
> contract.
Ah, so perhaps I simply conveyed the contract incorrectly. Here is what we
are bound to:
- The displayValue can be any String in the world
- The key must be one of the predefined enums, or null
Thus, the displayValue and the key have no relation to each other (spec wise
that is). Now, I'll give you a case where this makes sense. In Orkut, let's
say they have some radio buttons for your smoker value that look like this:
"unhip, rock it always, when i feel like it"
Which would translate into "no, yes, occasionally" (or whatever). Now, orkut
would probably use their container specific displayValues so that when
gadgets are showing information to the user the user knows what it is
talking about (in relation to the container) Thankfully, the gadgets also
have the keys though which means they know the Orkut's "unhip" is the same
as myspace's "no" without having a bunch of container specific logic in
their app.
>
> As per your previous response and my understanding of the API, this
> implementation fits the requirements (and darn it, I am a stickler for
> requirements). And while I can see the motivation for providing placeholder
> model classes and let developers fill in the blanks, I generally believe
> that such 'model' implementations should serve as reference implementations
> (go the extra mile, so to speak). All that said, I really do like your
> generics approach, much cleaner looking, etc. Let me play around with this
> today and respond with something this afternoon. Unless you or the team
> think the "contract enforcement" aspect is overboard, I really do urge the
> interested parties to keep this in the implementation.
I completely agree with your contract enforcement approach, I just think
that it doesn't exactly match the contract that the spec wanted to put out
their. Which would be a burden on containers.
So essentially, your coding styles are now aligned with Shindig, but I think
the concept behind it is still a little off. Again, I know this is super
complicated, and it is entirely my fault for not explaining it well enough.
Thank you for all the time you are spending on this!
- Cassie
>
> > Implementation POJO for opensocial.Enum.xx and opensocial.Message
> > -----------------------------------------------------------------
> >
> > Key: SHINDIG-124
> > URL: https://issues.apache.org/jira/browse/SHINDIG-124
> > Project: Shindig
> > Issue Type: New Feature
> > Components: Gadgets Server - Java
> > Reporter: Michael Papp
> > Attachments: fix-124-1-bug.patch.txt, fix-124-2-bug.patch.txt,
> fix-124-bug.patch.txt
> >
> >
> > Submitting proposed implementation for opensocial.Enum and
> opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker.
> This is a best guess pass as there are a number of ways of implementing
> this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway,
> hope this helps. If approach is right, then I can amend patch to include
> tests.
> > M. Papp
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
>
[jira] Commented: (SHINDIG-124) Implementation POJO for
opensocial.Enum.xx and opensocial.Message
Posted by "Michael Papp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12578346#action_12578346 ]
Michael Papp commented on SHINDIG-124:
--------------------------------------
Thank you for your assistance. Will make the needed changes to the editor and patch file. Yes, I was a bit confused by the Enum specification. Your explanation is most helpful indeed. I will take advantage of your insight and (and file format / patch file changes) and resubmit a patch later today. Thanks again!
> Implementation POJO for opensocial.Enum.xx and opensocial.Message
> -----------------------------------------------------------------
>
> Key: SHINDIG-124
> URL: https://issues.apache.org/jira/browse/SHINDIG-124
> Project: Shindig
> Issue Type: New Feature
> Components: Gadgets Server - Java
> Reporter: Michael Papp
> Attachments: fix-124-1-bug.patch.txt, fix-124-bug.patch.txt
>
>
> Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
> M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-124) Implementation POJO for
opensocial.Enum.xx and opensocial.Message
Posted by "Michael Papp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Papp updated SHINDIG-124:
---------------------------------
Attachment: fix-124-3-bug.patch.txt
This should do it. Attached is a patch that adds the Enum functionality without any pre-conditions -except- that attempts to set the displayValue null will throw a NullPointerException (as required by the OS 0.7 spec which states that this field must never be null). The Message Java model class is also added in this patch, as well as the attendant changes to the Person class for Enum. Hope this fits the bill.
> Implementation POJO for opensocial.Enum.xx and opensocial.Message
> -----------------------------------------------------------------
>
> Key: SHINDIG-124
> URL: https://issues.apache.org/jira/browse/SHINDIG-124
> Project: Shindig
> Issue Type: New Feature
> Components: Gadgets Server - Java
> Reporter: Michael Papp
> Attachments: fix-124-1-bug.patch.txt, fix-124-2-bug.patch.txt, fix-124-3-bug.patch.txt, fix-124-bug.patch.txt
>
>
> Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
> M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-124) Implementation POJO for
opensocial.Enum.xx and opensocial.Message
Posted by "Michael Papp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12580620#action_12580620 ]
Michael Papp commented on SHINDIG-124:
--------------------------------------
Hope you don't mind, Cassie - I added your list comments to the ticket below:
=======================================================================
---------- Forwarded message ----------
From: Cassie <do...@google.com>
To: shindig-dev@incubator.apache.org
Date: Wed, 19 Mar 2008 09:54:34 +0100
Subject: Re: [jira] Commented: (SHINDIG-124) Implementation POJO for opensocial.Enum.xx and opensocial.Message
Taking off the bug because this has turned into a longer thing..
On Tue, Mar 18, 2008 at 7:03 PM, Michael Papp (JIRA) <ji...@apache.org>
wrote:
>
> [
> https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12579963#action_12579963]
>
> Michael Papp commented on SHINDIG-124:
> --------------------------------------
>
> Hi Cassie - thanks again for your assistance with this code.
>
> 1. Will remove author tag
>
> 2. Here is my reasoning for the implementation:
> a. Rules for Enum:
> displayValue can never be null.
> Key may be null.
> if displayValue is one of the pre-defined constants, then
> displayValue == Key.toString().
I think this assumption is where things break. We shouldn't assume it will
be key.toString() it should be allowed to be an arbitrary string as well. If
we want to include a helper constructor that just takes in the enum only and
uses enum.toString for the displayValue then that is fine. it only needs to
be a helper though.
>
> if displayValue is not one of the pre-defined constants,
> then Key = null; displayValue = arbitrary string.
> b. Enum(Key key, String displayValue) is an unnecessary constructor, as
> either the Key is equivalent to the Value (String), or the Key != Value and
> the Key is null. Therefore, only the displayValue matters - not the Key.
> Still, for conformity, I could implement this constructor and throw away
> the Key.
if we go this route, you should really throw away the displayValue... not
the key. then you wouldn't need all of the lookup logic. however, as i
mentioned above, i think both should be preserved in this constructor. so
perhaps this:
public Enum(String displayValue) {
this(null, displayValue)
}
public Enum(T key) {
this(key, key.toString())
}
public Enum(T key, String displayValue) {
// set both
}
>
> c. The reverse look up on the enum Key properties enforces this
> contract.
Ah, so perhaps I simply conveyed the contract incorrectly. Here is what we
are bound to:
- The displayValue can be any String in the world
- The key must be one of the predefined enums, or null
Thus, the displayValue and the key have no relation to each other (spec wise
that is). Now, I'll give you a case where this makes sense. In Orkut, let's
say they have some radio buttons for your smoker value that look like this:
"unhip, rock it always, when i feel like it"
Which would translate into "no, yes, occasionally" (or whatever). Now, orkut
would probably use their container specific displayValues so that when
gadgets are showing information to the user the user knows what it is
talking about (in relation to the container) Thankfully, the gadgets also
have the keys though which means they know the Orkut's "unhip" is the same
as myspace's "no" without having a bunch of container specific logic in
their app.
>
> As per your previous response and my understanding of the API, this
> implementation fits the requirements (and darn it, I am a stickler for
> requirements). And while I can see the motivation for providing placeholder
> model classes and let developers fill in the blanks, I generally believe
> that such 'model' implementations should serve as reference implementations
> (go the extra mile, so to speak). All that said, I really do like your
> generics approach, much cleaner looking, etc. Let me play around with this
> today and respond with something this afternoon. Unless you or the team
> think the "contract enforcement" aspect is overboard, I really do urge the
> interested parties to keep this in the implementation.
I completely agree with your contract enforcement approach, I just think
that it doesn't exactly match the contract that the spec wanted to put out
their. Which would be a burden on containers.
So essentially, your coding styles are now aligned with Shindig, but I think
the concept behind it is still a little off. Again, I know this is super
complicated, and it is entirely my fault for not explaining it well enough.
Thank you for all the time you are spending on this!
- Cassie
===============================================================================
OK, I am almost done with the Generics approach and it is *indeed* looking nice. I also want to thank you for putting up with my slow uptake on the contract semantics inherent in the spec. I guess it is a matter of coming from the world of operating systems and app frameworks where the spec is cast in concrete and you can modify the behavior in your own code. This spec is a bit more like "here are the calls, implement them as fits your needs." That is fine, as long as everyone tries to remain consistent. The use case that bothers me is when the Key is set to "NO" and the displayValue is set to "yes". The developer (and end user) will obviously get different answers dependent on which field is queried, and logic which depends on a consistent behavior will not work. Of course it behooves the implementor to "make it work" by keeping the two in some sort of sync. OTOH, a standard reference semantic behavior helps avoid those sorts of mistakes. Skimming the various lists on OpenSocial quickly turns up developers complaints about fragmentation and/or a standard without common semantics. That said, I would strongly recommend giving a some formal semantics to the Shindig implementation, and letting developers change the behavior in the .js or Java layers by subclassing or modifying code. OK, soapbox off.
I will have a new patch for you tomorrow, very clean and simple. Thank you for the Generics idea - quite brilliant.
> Implementation POJO for opensocial.Enum.xx and opensocial.Message
> -----------------------------------------------------------------
>
> Key: SHINDIG-124
> URL: https://issues.apache.org/jira/browse/SHINDIG-124
> Project: Shindig
> Issue Type: New Feature
> Components: Gadgets Server - Java
> Reporter: Michael Papp
> Attachments: fix-124-1-bug.patch.txt, fix-124-2-bug.patch.txt, fix-124-bug.patch.txt
>
>
> Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
> M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-124) Implementation POJO for
opensocial.Enum.xx and opensocial.Message
Posted by "Michael Papp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Papp updated SHINDIG-124:
---------------------------------
Attachment: fix-124-1-bug.patch.txt
Add opensocial.Message class to org.apache.shindig.social.opensocial.model to patch as well as some mods to opensocial.Enum and subclasses.
> Implementation POJO for opensocial.Enum.xx and opensocial.Message
> -----------------------------------------------------------------
>
> Key: SHINDIG-124
> URL: https://issues.apache.org/jira/browse/SHINDIG-124
> Project: Shindig
> Issue Type: New Feature
> Components: Gadgets Server - Java
> Reporter: Michael Papp
> Attachments: fix-124-1-bug.patch.txt, fix-124-bug.patch.txt
>
>
> Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
> M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-124) Implementation POJO for
opensocial.Enum.xx and opensocial.Message
Posted by "Michael Papp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Papp updated SHINDIG-124:
---------------------------------
Attachment: fix-124-2-bug.patch.txt
Modified as per request on formatting and patch file format.
Modified Enum and subclasses as per Cassie's instructions (I believe). If this fulfills requirements, I will submit a new patch with changes to the Person class and unit tests (or after patch is applied).
> Implementation POJO for opensocial.Enum.xx and opensocial.Message
> -----------------------------------------------------------------
>
> Key: SHINDIG-124
> URL: https://issues.apache.org/jira/browse/SHINDIG-124
> Project: Shindig
> Issue Type: New Feature
> Components: Gadgets Server - Java
> Reporter: Michael Papp
> Attachments: fix-124-1-bug.patch.txt, fix-124-2-bug.patch.txt, fix-124-bug.patch.txt
>
>
> Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
> M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Resolved: (SHINDIG-124) Implementation POJO for
opensocial.Enum.xx and opensocial.Message
Posted by "Paul Lindner (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Paul Lindner resolved SHINDIG-124.
----------------------------------
Resolution: Fixed
Reviewed - This patch looks good. Thanks for shepherding it through.
> Implementation POJO for opensocial.Enum.xx and opensocial.Message
> -----------------------------------------------------------------
>
> Key: SHINDIG-124
> URL: https://issues.apache.org/jira/browse/SHINDIG-124
> Project: Shindig
> Issue Type: New Feature
> Components: Gadgets Server - Java
> Reporter: Michael Papp
> Attachments: fix-124-1-bug.patch.txt, fix-124-2-bug.patch.txt, fix-124-3-bug.patch.txt, fix-124-bug.patch.txt
>
>
> Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
> M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-124) Implementation POJO for
opensocial.Enum.xx and opensocial.Message
Posted by "Michael Papp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Papp updated SHINDIG-124:
---------------------------------
Summary: Implementation POJO for opensocial.Enum.xx and opensocial.Message (was: Implementation POJO for opensocial.Enum)
Adding opensocial.Message to patch as well.
> Implementation POJO for opensocial.Enum.xx and opensocial.Message
> -----------------------------------------------------------------
>
> Key: SHINDIG-124
> URL: https://issues.apache.org/jira/browse/SHINDIG-124
> Project: Shindig
> Issue Type: New Feature
> Components: Gadgets Server - Java
> Reporter: Michael Papp
> Attachments: fix-124-bug.patch.txt
>
>
> Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
> M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Updated: (SHINDIG-124) Implementation POJO for
opensocial.Enum
Posted by "Michael Papp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Michael Papp updated SHINDIG-124:
---------------------------------
Attachment: fix-124-bug.patch.txt
Here are the .java files
> Implementation POJO for opensocial.Enum
> ---------------------------------------
>
> Key: SHINDIG-124
> URL: https://issues.apache.org/jira/browse/SHINDIG-124
> Project: Shindig
> Issue Type: New Feature
> Components: Gadgets Server - Java
> Reporter: Michael Papp
> Attachments: fix-124-bug.patch.txt
>
>
> Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
> M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-124) Implementation POJO for
opensocial.Enum.xx and opensocial.Message
Posted by "Michael Papp (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12579963#action_12579963 ]
Michael Papp commented on SHINDIG-124:
--------------------------------------
Hi Cassie - thanks again for your assistance with this code.
1. Will remove author tag
2. Here is my reasoning for the implementation:
a. Rules for Enum:
displayValue can never be null.
Key may be null.
if displayValue is one of the pre-defined constants, then displayValue == Key.toString().
if displayValue is not one of the pre-defined constants, then Key = null; displayValue = arbitrary string.
b. Enum(Key key, String displayValue) is an unnecessary constructor, as either the Key is equivalent to the Value (String), or the Key != Value and the Key is null. Therefore, only the displayValue matters - not the Key. Still, for conformity, I could implement this constructor and throw away the Key.
c. The reverse look up on the enum Key properties enforces this contract.
As per your previous response and my understanding of the API, this implementation fits the requirements (and darn it, I am a stickler for requirements). And while I can see the motivation for providing placeholder model classes and let developers fill in the blanks, I generally believe that such 'model' implementations should serve as reference implementations (go the extra mile, so to speak). All that said, I really do like your generics approach, much cleaner looking, etc. Let me play around with this today and respond with something this afternoon. Unless you or the team think the "contract enforcement" aspect is overboard, I really do urge the interested parties to keep this in the implementation.
> Implementation POJO for opensocial.Enum.xx and opensocial.Message
> -----------------------------------------------------------------
>
> Key: SHINDIG-124
> URL: https://issues.apache.org/jira/browse/SHINDIG-124
> Project: Shindig
> Issue Type: New Feature
> Components: Gadgets Server - Java
> Reporter: Michael Papp
> Attachments: fix-124-1-bug.patch.txt, fix-124-2-bug.patch.txt, fix-124-bug.patch.txt
>
>
> Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
> M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-124) Implementation POJO for
opensocial.Enum.xx and opensocial.Message
Posted by "Cassie Doll (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12578242#action_12578242 ]
Cassie Doll commented on SHINDIG-124:
-------------------------------------
Hey Michael -
Thanks for the patch! Unfortunately though I need you to make the diff for the patch from within your shindig directory. Right now the patch has lines that look like this: --- C:/work/OpenSocial_Project/projects/piczo_sd/src/java/gadgets/... which means that when I patch it it creates a new directory called "C:" with a work dir inside etc etc.
Please refer to the patch instructions on our website if I didn't explain that well enough.
Also, some quick things I noticed so far:
- we use 2 space indents, so you should probably change your editor to use that standard and re-indent those files
- and the enum logic is a little off, i know this is pretty confusing in the spec so let me try to explain it a little better
So, the smoker field in the Person class needs to return an Enum object. (That's pretty easy: private Enum smoker)
Then, the smoker.getKey() should equal NO, YES, SOCIALLY, etc, or null.
Lastly, the smokers displayValue() should return something usable as a string. For now this is usually just the non-caps case, "no", "yes", "socially" etc. It can be any string though. Especially if the getKey() returned null.
This handles the following cases:
- pretend social network A allows you to set your smoking preference. On their site you can say "yes, no, twice a week". When contructing opensocial objects this would correspond to:
-- smoker : {key: YES, displayValue: yes }
-- smoker : {key: NO, displayValue: no }
-- smoker : {key: null, displayValue: twice a week }
or if the user spoke german and smoked:
-- smoker : {key: YES, displayValue: ya }
The gadget can always use person.getField('smoker').getDisplayValue() when they just want to display something on the screen. If they want to generate smoker stats then they can use the key value to guarantee consistency across containers and languages.
This new explanation I think changes what you have just a little. Let me know if you have any more questions. Thanks!
- Cassie
> Implementation POJO for opensocial.Enum.xx and opensocial.Message
> -----------------------------------------------------------------
>
> Key: SHINDIG-124
> URL: https://issues.apache.org/jira/browse/SHINDIG-124
> Project: Shindig
> Issue Type: New Feature
> Components: Gadgets Server - Java
> Reporter: Michael Papp
> Attachments: fix-124-1-bug.patch.txt, fix-124-bug.patch.txt
>
>
> Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
> M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (SHINDIG-124) Implementation POJO for
opensocial.Enum.xx and opensocial.Message
Posted by "Cassie Doll (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/SHINDIG-124?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12579817#action_12579817 ]
Cassie Doll commented on SHINDIG-124:
-------------------------------------
Hey Michael -
Thanks for redoing all of this. I only have two comments this time:
1. We don't use author tags in Shindig.
2. I was picturing something a little simpler for the design of these classes. Namely something like this:
public interface Enum {
public String getDisplayValue();
public Object getKey(); // as long as getKey().toString() is json then everything is cool because they are just enums than that is fine
}
public final class Drinker extends AbstractGadgetData implements Enum {
private String displayValue;
private Key key;
public enum Key {
HEAVILY, NO
}
public Drinker(String displayValue) {
this(null, displayValue);
}
public Drinker(Key key, String displayValue) {
this.key = key;
this.displayValue = displayValue;
}
// setters and getters
}
So then the java code can make any sort of Drinker it wants, it is only a simple pojo and it matches the js interface. You could even go simpler and use generics to achieve this:
public class Enum<T> extends AbstractGadgetData {
public String getDisplayValue();
public T getKey();
// basic constructor + setters and getters
public enum Drinker { yes, no, etc etc}
}
// In Person.java
private Enum<Drinker> drinker;
private Enum<Smoker> smoker;
What do you think?
(I prefer the generics one - less code and such)
Thanks for your help!
- Cassie
> Implementation POJO for opensocial.Enum.xx and opensocial.Message
> -----------------------------------------------------------------
>
> Key: SHINDIG-124
> URL: https://issues.apache.org/jira/browse/SHINDIG-124
> Project: Shindig
> Issue Type: New Feature
> Components: Gadgets Server - Java
> Reporter: Michael Papp
> Attachments: fix-124-1-bug.patch.txt, fix-124-2-bug.patch.txt, fix-124-bug.patch.txt
>
>
> Submitting proposed implementation for opensocial.Enum and opensocial.Enum.Drinker, opensocial.Enum.Gender, and opensocial.Enum.Smoker. This is a best guess pass as there are a number of ways of implementing this, and opensocial 0.7 spec for the getKey() is rather nebulous. Anyway, hope this helps. If approach is right, then I can amend patch to include tests.
> M. Papp
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.