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

[Andrew Azores]

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 :)

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.

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.

>>
>>>
>>> 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
>>
>


-- 
Thanks,

Andrew Azores