[rfc][icedtea-web] rememberable dialogues general solution - impl part 1 - safe types returned from getUserResponse

[Jiri Vanek]

On 06/08/2015 04:04 PM, Andrew Azores wrote:
> Hi,
>
> On 08/06/15 06:18 AM, Jiri Vanek wrote:
>> On 06/05/2015 05:36 PM, Andrew Azores wrote:
>>> Hi! Generally liking where this patch is going.
>>>
>>> Jiri Vanek <jvanek at ...> writes:
>>>
>>>>
>>>> hi!
>>>>
>>>> So this is first part.
>>>> Tho most nasty and error prone.
>>>>
>>>> It is changing type of securityMEssage.response from Object to regular
>>> interface, and is providing
>>>> useful implementations of this interface.
>>>>
>>>> The legacy object was integer at first, and was used to preselect button,
>>> and tro return id of
>>>> pressed button. LAter was missuesd and returned yes/no/always/never even
>>> later sanbox was added and
>>>> last drop was complex result about shortcuts.
>>>
>>> I much prefer your new approach, obviously much better than ints and Object[]s.
>>>
>>>>
>>>> In this patch, there are few wrappers for types of yAnNS, which contains
>>> also remember logic. This
>>>> will be fixed in another changeset, and those wrappers will be gone.
>>>
>>> Does this mean the Yes, No, YesNo, YesNoCancel, etc. classes? And "Garbage"?
>>
>> Garabge for sure:)
>>
>> But not so YEs, No, YesNo - they are what dfialog can return, and what is returning. What you would like to replace them with?
>> If dialog returns more htne just yes/nos/sandbox/any_button then they can be rused in same way as AccessWarningPaneComplexReturn do.
>>
>> Thoughts?
>
> I don't know, I don't have anything in particular in mind that this might be replaced with. I understand what these types signify but it just seems like a cumbersome way to have to define them, but I'm not sure how else it might be done better. Could you perhaps rename all the Yes/YesNo/YesNoCancel classes and add "Result" or something similar to their names? "new Yes()" is a very odd looking snippet :)

Nope. They all are implementing DialogResult. Thats better then naming convention, isnt it?
>
> It also seems to me like there's no sense to individual instances of these... "Yes a = new Yes()" and "Yes b = new Yes()" are just two different instances of stateless things that should always compare as equal, right? The other classes eg YesNo have factory methods for obtaining instances anyway, so why not standardize each of these classes on using factory methods and have each factory method return a reference to the same object every time, whenever possible (ie just not for the readValue case)? In other words, define singletons for each value of each return type. I think this might make the intention of these types a little clearer, and also might help avoid gotchas like trying to compare the returns of two
> dialogs.

Well.. no. You can not guarantee for any possible DialogResult, that it will behave like that. The part where it canbe guaranteed -  is hte enum - YES, NO...SANDBOX... So it is "singleton as you are suggesting"
The classes around, have a bit different  - especially to guarantee that nothing else willl be passed in.

But yo are deffinitley right with missing equals. I already spotted it at friday, when I was writing unittests for it. I will add equals+hash codes. They are  trivbial anyway all except AccessWarningPaneComplexReturn(which is still simpel enough)
>
> If you didn't have to define each of these type as extending a common base type then it would be much nicer to just make each of these into an enum, but unfortunately that's not possible. I don't think that the lazy init matters here and thread safety doesn't seem like a concern either, though, so private static final instances with factory methods to return references to them still seems alright to me.

I was thinking about singletons inside YesNo.no()/yes() and similar. I abandoned them, because  you can read/write those to/from string. So it seemed better to me just return new instance instead of switch, which will determine instance.
again, this is bringing troubles in case of more compelx types.

>
>>>
>>>>
>>>> Approach of this patch i slightly different, less verbose, with a lot of
>>> shared code and different
>>>> typesafety. I hope it is generally better. If somebody found tech-review's
>>> solution better, or
>>>> generally have better ideas, pelase let me know.
>>>>
J.
>>>>
>>>> Attachment (makeTypesOfSecurityDialogMessageReturnValueSafe.patch):
>>> text/x-patch, 84 KiB
>>>
>>>
>>> Thanks,
>>>
>>> Andrew Azores
>>>
>>
>
>