This version (2017/05/27 13:44) is a draft.
Approvals: 0/1

[09:38:47] <temporal_> brunoais hi

[09:38:50] <temporal_> still need help ?

[09:43:16] <brunoais> temporal_, yes please

[09:43:27] <temporal_> sure

[09:43:38] <temporal_> you need to have @VertxGen I think

[09:43:50] <brunoais> I want to understand the best way to deal with vertx3-gen

[09:43:52] <temporal_> I need to look at the deisgn you have with intreface

[09:44:28] <brunoais> temporal_, https://github.com/vert-x3/vertx-web/pull/446

[09:44:39] <temporal_> which class is raining problem ?

[09:44:53] <brunoais> I did a workaround

[09:45:00] <brunoais> the code is ugly now

[09:45:38] <temporal_> you said that you have issues with concrete interface

[09:45:41] <brunoais> temporal_, do you want me to reset to before I made the workaround for you to take a look?

[09:45:42] <brunoais> Yes

[09:45:44] <temporal_> does the code currently compiles ?

[09:45:50] <temporal_> yes it would be best to help you

[09:45:51] <brunoais> with the workaround yes

[09:45:57] <brunoais> OK. Pls wait

[09:49:31] <brunoais> It is complaining here: https://github.com/vert-x3/vertx-web/pull/446/files#diff-d0cc17dc348c58b0a095e2017d85c542R9

[09:49:58] <brunoais> vertx-web/src/main/java/io/vertx/ext/web/LanguageHeader.java:[9,8] Could not generate model for io.vertx.ext.web.LanguageHeader: A concrete interface cannot extend more than one concrete interfaces

[09:50:11] <brunoais> temporal_, ^

[09:50:38] <temporal_> you should do

[09:50:46] <temporal_> annotate

[09:51:24] <brunoais> Annotate with what?

[09:51:29] <temporal_> I'm looking

[09:51:32] <brunoais> OK

[09:52:29] <temporal_> is there a real benefit to have “ParsedHeaderValue” interface ?

[09:52:59] <brunoais> There is

[09:53:00] <temporal_> anyway

[09:53:15] <temporal_> I think that if you do @VertxGen(concrete = false) on ParsedHeaderValue it should resolve the problem

[09:53:16] <brunoais> It is part of a hierarchy of parsed headers

[09:55:17] <temporal_> and you cannot annotated with @VertxGen a class

[09:55:25] <temporal_> but it is not taken in account

[09:55:33] <temporal_> i.e ParsableHeaderValue

[09:55:39] <temporal_> you can remove @VertxGen annotation it is useless

[09:55:45] <brunoais> It is not

[09:55:48] <brunoais> I need it

[09:55:59] <brunoais> there are places where that interface is used

[09:55:59] <temporal_> then it shojlld be an interface

[09:56:02] <temporal_> and not in impl package

[09:57:01] <brunoais> It is an interface

[09:57:09] <brunoais> and it is not in the impl package

[09:57:45] <temporal_> I can see

[09:57:45] <temporal_> [email protected]

[09:57:45] <temporal_> +public class ParsableHeaderValue implements ParsedHeaderValue {

[09:57:47] <temporal_> +

[09:58:02] <brunoais> Oh, that one

[09:58:07] <brunoais> then that's a mistake

[09:58:11] <temporal_> I'm going to fork the code

[09:58:15] <temporal_> to try to make it compile for you :-)

[09:58:18] <temporal_> it will be simpler maybe

[09:58:38] <brunoais> I had fixed that before

[09:58:48] <brunoais> when I made the reset I forgot to add that fix

[09:58:50] <brunoais> anyway

[09:59:01] <brunoais> vertx-web/src/main/java/io/vertx/ext/web/ParsedHeaderValue.java:[82,45] Could not generate model for <T>findMatchedBy(java.lang.Iterable<T>): Type parameter bound not supported io.vertx.ext.web.ParsedHeaderValue

[09:59:13] <brunoais> temporal_, ^

[09:59:29] <brunoais> https://github.com/vert-x3/vertx-web/pull/446/files#diff-dd3eb514d42e59d0a946da67227f14b4R82

[10:00:07] <temporal_> yes you cannot use Optional

[10:00:16] <temporal_> and also extends

[10:00:28] <brunoais> :S

[10:00:38] <brunoais> I can understand Optional

[10:00:42] <brunoais> But why not extends?

[10:01:08] <brunoais> If I remove extends, I'll lose all that typecheck from java

[10:01:10] <temporal_> because some languags have different generic handling than java

[10:01:19] <temporal_> you can annotate with @GenIgnore

[10:01:40] <temporal_> but then it will not be availlable in groovy or javascript

[10:01:42] <temporal_> etc…

[10:01:53] <brunoais> Can it be done so that other languages ignore the generics?

[10:02:12] <brunoais> I mean, when translating to other languages, the generics are not used and a cast is built?

[10:02:30] <temporal_> no

[10:02:33] <temporal_> the method is ignored

[10:02:38] <temporal_> if it is annotated with @GenIgnore

[10:02:53] <temporal_> I totally understand what you want to do

[10:02:56] <brunoais> Then it is not good because other parts of the program use it

[10:03:07] <temporal_> and you should use Collection and not Iterable

[10:03:25] <temporal_> is it intended to be a user method ?

[10:03:29] <temporal_> or for the framework itself ?

[10:03:52] <brunoais> Oh! Does it support Collection<T>? When I read the manual I only saw Set<T> and List<T>

[10:04:00] <brunoais> vert.x only

[10:04:07] <temporal_> the method should be

[10:04:07] <temporal_> ParsedHeaderValue findMatchedBy(List<ParsedHeaderValue> matchTries);

[10:04:08] <brunoais> but if code outside wants to use it, it's fine

[10:04:20] <brunoais> Yeah…

[10:04:23] <brunoais> casts FTW

[10:04:24] <temporal_> it's not a big deal

[10:04:28] <brunoais> *sigh*

[10:04:29] <temporal_> what you could do is

[10:04:37] <temporal_> have this method on the implemnetaiton class

[10:04:37] <temporal_> and hide it from interface

[10:04:49] <temporal_> i.e on ParsableHeaderValue

[10:04:51] <brunoais> I already tried it

[10:05:10] <brunoais> I had to go back because most “keepers” require storing only interfaces

[10:05:15] <temporal_> I see

[10:05:50] <temporal_> can't you override that using covariant return types ?

[10:05:59] <temporal_> i.e you get one initial type that is Impl

[10:06:07] <temporal_> and then Imp returns other Impl

[10:06:46] <brunoais> That's what the <T extends X> was doing there

[10:06:47] <temporal_> I willI am going to try

[10:07:28] <brunoais> There's a tree-like organization of data there

[10:08:22] <brunoais> ParsedHeaderValue ← LanguageHeader, MIMEHeader

[10:08:40] <brunoais> There are 2 ParsedHeaderValue, 1 LanguageHeader and 1 MIMEHeader

[10:08:52] <brunoais> total of 4 ParsedHeaderValue

[10:09:15] <temporal_> can't we use a single one ?

[10:11:10] <brunoais> How do you get the component of the Language header? Makes no sense

[10:11:18] <brunoais> *Accept-Language

[10:11:37] <temporal_> I got it compile!

[10:11:46] <temporal_> but using @GenIgnore

[10:11:50] <temporal_> will try to move these methods away

[10:11:51] <brunoais> T.T

[10:12:27] <brunoais> I also gain a LOT with polymorphism here because, in most places, I only care it is a ParsedHeaderValue

[10:12:52] <brunoais> But then, in certain specific places, I do care to get the correct kind pf ParsedHeaderValue

[10:14:19] <temporal_> that being said

[10:14:23] <brunoais> I also want to expose the parsed headers to the user

[10:14:28] <temporal_> we could remove the upper bound

[10:14:40] <brunoais> Casts FTW, then

[10:14:41] <temporal_> maybe that T extends ParsedHeaderValue

[10:14:48] <temporal_> no it would not be a cast :-)

[10:14:54] <temporal_> it would be more error prone

[10:15:00] <temporal_> i.e

[10:15:01] <temporal_> <T> T findMatchedBy(Collection<T> matchTries);

[10:15:02] <temporal_> would work

[10:15:12] <brunoais> but then I have to cast it

[10:15:19] <brunoais> T can be Object

[10:15:21] <temporal_> in the implementation ?

[10:15:26] <brunoais> yes

[10:15:27] <temporal_> I think that is not a problem

[10:15:39] <temporal_> the problem is more that somebody can use any object

[10:15:46] <temporal_> and does not see he only can use ParsedHeaderValue

[10:15:59] <brunoais> yeah

[10:16:07] <temporal_> other solution would be to duplicate the methods

[10:16:11] <temporal_> have findMatchedBy with different container type

[10:16:26] <brunoais> not polymorphic, I don't like it

[10:16:32] <temporal_> i.e

[10:16:37] <temporal_> findMimeHeaderMatchedBy

[10:16:54] <brunoais> yep, not polymorphic

[10:17:22] <temporal_> is this method intented to be used by only vertx web ?

[10:17:27] <temporal_> or does the user care about it ?

[10:17:27] <brunoais> yes

[10:17:46] <brunoais> This is intended for internal use only

[10:17:58] <brunoais> but I don't care if the user wants to adventure in it

[10:18:21] <brunoais> It is a safe and idemputent method

[10:21:33] <temporal_> I think then we can keep it

[10:21:34] <temporal_> and add

[10:21:38] <temporal_> for internal usage

[10:21:41] <temporal_> something like that

[10:21:53] <temporal_> and I remove the optional part because it makes it too serious :-)

[10:23:00] <brunoais> OK

[10:24:23] <brunoais> temporal_, I had this comment there:

[10:24:24] <brunoais> @note Effective declaration:

[10:24:24] <brunoais> * <T extends ParsedHeaderValue> T findMatchedBy(Iterable<T> matchTries);

[10:24:30] <brunoais> Does it make sense?

[10:24:36] <brunoais> It is part of the javadoc

[10:24:51] <temporal_> I'm keeping the extends

[10:24:55] <temporal_> and the method is annotated

[10:24:56] <temporal_> @GenIgnore

[10:25:02] <temporal_> so you can keep the bounds

[10:25:22] <temporal_> I need to think if <T extends ParsedHeaderValue> makes sense or not

[10:25:30] <temporal_> it is just an upper bound

[10:25:46] <temporal_> used this way it is not harfmul

[10:25:52] <temporal_> what is harmful is

[10:26:02] <temporal_> Collection<? extends> and Collection<? super>

[10:26:25] <temporal_> so I nee dto see how this would be supported in Scala , Ceylon

[10:26:37] <temporal_> and translated

[10:28:42] <brunoais> temporal_, In Scala, you use [T <: MyClass]

[10:29:04] <temporal_> I don't do the scala part

[10:29:08] <temporal_> and jochen is developing it

[10:29:15] <brunoais> OK

[10:30:00] <brunoais> IMO, if the language does not support it (javascript for example) it should just be ignored

[10:30:22] <brunoais> or if the language only supports it partially (without super or extends), it should just remove the generic and use the cast instead

[10:31:02] <temporal_> for js it's not a problem

[10:31:23] <brunoais> For the <T super X>, just assume an equivalent to Object.

[10:31:25] <temporal_> but <T> has a very specific meaning

[10:31:27] <temporal_> for vertx

[10:31:37] <temporal_> it means that it is either

[10:31:40] <brunoais> For <T extends X> Just cast to X.

[10:31:44] <temporal_> a basic type (string, integer), or json

[10:31:49] <brunoais> OH!!!!

[10:31:54] <temporal_> so T extends @VertxGen type

[10:32:05] <brunoais> Oh wait

[10:32:06] <temporal_> means that we should be able to unwrap/rewrap objects

[10:32:17] <brunoais> And you still are

[10:33:12] <brunoais> If you take in the type at extension (ParsedHeaderValue) and wrap it as that, it is supposed to work well, at least in the JVM

[10:33:23] <brunoais> but I dunno how their compilers work, unfortunately

[10:33:26] <temporal_> when you have

[10:34:02] <temporal_> Route route(String path);

[10:34:07] <temporal_> in Router

[10:34:09] <temporal_> in javascript

[10:34:13] <temporal_> the returned Route

[10:34:21] <temporal_> is wrapped with a javascript Route object

[10:34:24] <temporal_> that is code generated

[10:34:37] <temporal_> so if we do that for T extends Route

[10:34:45] <temporal_> it means we can rewrap to Route at most

[10:34:57] <temporal_> if we have RouteWhatever extends Route

[10:35:09] <temporal_> it means we need to be able to find the wrapper of RouteWhatever dynamically

[10:35:26] <temporal_> it means throwing more complexity too

[10:35:41] <brunoais> Dynamically?

[10:35:53] <brunoais> It can be found at compile time

[10:36:24] <brunoais> Otherwise, how would java handle generics if they are lost in the processed bytecode?

[10:36:45] <brunoais> (they are only there as metadata only accessible using reflection)

[10:37:58] <temporal_> you can find it for Route

[10:38:24] <temporal_> https://github.com/vietj/vertx-web/tree/pocs/issues/442

[10:40:48] <brunoais> That URl is not pointing to the right thing…

[10:40:59] <brunoais> oh

[10:41:00] <brunoais> it is

[10:41:11] <brunoais> Hum…

[10:44:06] <temporal_> sorry

[10:44:17] <temporal_> ah it us yes

[10:44:23] <temporal_> maybe I introduced bug when I remove optional

[10:44:27] <temporal_> I rmeoved also EMPTY = “”

[10:44:30] <temporal_> check

[10:44:43] <brunoais> EMPTY was to have a unique pointer

[10:45:11] <brunoais> if you remove Optional, you have to add the nulls

[10:46:13] <brunoais> @→ private void addParameter(String key, String value) {

[10:46:21] <brunoais> value = “”;

[10:46:47] <brunoais> Bad idea. That will reuse the global static reference of the empty string

[10:47:05] <brunoais> if that's used elsewhere, it will “blow up” with bad bugs

[10:47:34] <brunoais> * if the empty string is used

[10:47:37] <temporal_> you don't need optional pointer

[10:51:32] <brunoais> temporal_, what do you mean by “optional pointer”?

[10:53:49] <brunoais> temporal_, how do I distinguish the empty string from being empty?

[10:55:00] <temporal_> can you update the branch with these ammends you are saying ?

[10:56:33] <brunoais> I need to leave work first :(

[10:56:39] <brunoais> I have to start working now

[10:57:10] <brunoais> temporal_, Are you available in 9 hours?

[11:03:59] <temporal_> maybe :-)

[11:04:01] <temporal_> what time ?

[11:04:11] <temporal_> 8pm I will be probably at lunch

[11:04:15] <temporal_> so it would be later

[11:04:25] <temporal_> like after 9pm

[11:04:29] <temporal_> one hour later

[11:53:42] <klmn> hello guys, is this a good place for questions?

[13:46:39] <brunoais> temporal_, 20h GMT+1?

[13:54:36] <temporal_> brunoais Paris time

[14:41:29] <brunoais> OK. I'll coordinate that

[21:07:39] <brunoais> temporal_, I'm back for a while until I go dinning (dunno when)

[21:32:23] <temporal_> hi brunoais

[21:32:35] <brunoais> hey

[21:33:28] <temporal_> I was taking my daughter to bed

[21:33:36] <temporal_> on friday night it lasts a bit more

[21:34:33] <brunoais> It's OK

[21:34:35] <temporal_> so what's left ?

[21:34:54] <brunoais> I'm unsure

[21:35:07] <brunoais> Take a look at the PR and see if it looks OK

[21:35:42] <brunoais> It is passing all the tests, including mine

[21:35:59] <temporal_> did you merge with what I did ?

[21:36:20] <temporal_> which pr is it ?

[21:37:00] <temporal_> ah yes I see it

[21:37:05] <temporal_> there are lot of commits lol

[21:37:20] <temporal_> concerning polyglot I think it's fine

[21:37:57] <temporal_> I think that the rest of the review for the feature is to Paulo to do

[21:38:09] <temporal_> although I can make comments related to the style

[21:38:18] <temporal_> like you had an == with EMPTY = “”

[21:38:27] <temporal_> that I replaced with .isEmpty()

[21:38:38] <temporal_> so I would have commented on that if I would not have corrected it

[21:38:58] <temporal_> so I think there is still need to be reviewed

[21:39:05] <temporal_> (all PRs do)

[21:39:11] <temporal_> but at least now it is polyglot

[21:43:37] <temporal_> I'm doing some review now

[21:45:09] <brunoais> temporal_, I did a comment, though

[21:45:25] <brunoais> you deleted it

[21:45:53] <brunoais> It read

[21:45:55] <brunoais> “ unique string object reference” [21:46:45] <temporal_> ok [21:46:46] <temporal_> put it back [21:46:52] <temporal_> it was not on purpose I think [21:46:55] <temporal_> ah [21:47:03] <temporal_> I think it was ok actually to remove it (Iremember now) [21:47:43] <brunoais> I'm still wondering about it [21:51:01] <brunoais> temporal_, How do you distinguish a set empty string from an unset value? [21:51:18] <temporal_> what do you mean ? [21:51:38] <brunoais> let me check… [21:53:24] <brunoais> temporal_, what if I require the parameter value to be an empty string and I got a parameter with no value? [21:53:57] <temporal_> which line of which file are we discussing ? [21:54:32] <brunoais> ParsableHeaderValue.java:82 [21:56:18] <temporal_> before it was compared to empty string “” [21:56:19] <temporal_> with == [21:56:30] <temporal_> isEmpty() is like comparing to empty string [21:56:45] <brunoais> It was being compared to a specific string instance ;) [21:57:01] <brunoais> It was a unique object reference [21:57:40] <temporal_> yes why ? [21:57:43] <temporal_> nobody does that [21:57:54] <temporal_> and it is not a specific instance [21:57:57] <temporal_> it's an interned value [21:58:05] <temporal_> interned string [21:58:07] <brunoais> It's not [21:58:10] <brunoais> do that test yourself [21:58:14] <temporal_> you did new String(“”) ? [21:58:16] <brunoais> yes [21:58:19] <temporal_> what was the purpose ? [21:58:21] <brunoais> yes [21:58:32] <brunoais> Hence the comment :) [21:58:46] <brunoais> “ unique string object reference”

[21:59:27] <temporal_> ah yes I see it

[21:59:30] <temporal_> but why doing that ?

[21:59:48] <temporal_> for the sake of ?

[21:59:58] <brunoais> null could both mean “I don't have that” and “I'm storing a null ^^”

[22:00:15] <brunoais> asking if a Map has a key costs about as much as doing just a get

[22:00:30] <temporal_> in this case it is very neglectable I think

[22:00:40] <brunoais> although a get() is O(1) (let's assume that), it takes some time

[22:00:40] <temporal_> given we are using regex all the way :-)

[22:00:48] <temporal_> so it's a marker ?

[22:00:57] <temporal_> then this marker should be I think a different string like

[22:00:57] <brunoais> Yeah

[22:01:07] <temporal_> “@[email protected]

[22:01:14] <brunoais> Feel free to

[22:01:18] <temporal_> and be kept internal

[22:01:27] <brunoais> “@[email protected]” ←

[22:01:29] <brunoais> no

[22:01:32] <brunoais> make it unique

[22:01:38] <brunoais> that way it cannot be cheated

[22:01:42] <temporal_> maybe but not exposed in an interface

[22:01:54] <brunoais> I c…

[22:02:15] <temporal_> but I don't mind the containsKey / get

[22:02:25] <temporal_> one thing you could use maybe instead is

[22:02:30] <temporal_> computeIfAbsent

[22:02:42] <temporal_> and you return a specific String instance

[22:03:30] <temporal_> are these stored ?

[22:03:34] <temporal_> explain me :-)

[22:03:34] <brunoais> Hum… that can do the trick…

[22:03:42] <brunoais> I g2g now

[22:03:52] <brunoais> we can continue this discussion as I finish, OK?

[22:04:01] <brunoais> maybe in 30 mins…. worst case 1h

[22:04:14] <temporal_> let's say 11pm ?

[22:04:18] <temporal_> i.e 55mn

[22:04:28] <brunoais> I think so

[22:04:35] <temporal_> I've done comments on the source code

[22:04:37] <temporal_> review

[22:40:24] <brunoais> temporal_, back

[22:41:16] <brunoais> I thought about it. computeIfAbsent() doesn't work because I need to convert an execution into a true/false return.

[22:42:01] <brunoais> The callback is a method on its own and there's no easy/direct way of getting its return value or return anything at al

[22:42:03] <brunoais> *all

[23:15:55] <brunoais> temporal_?

[23:16:05] <temporal_> hi again

[23:16:16] <brunoais> I made some comments back

[23:16:23] <temporal_> ok

[23:16:28] <temporal_> I will process them tomorrow :-)

[23:16:35] <temporal_> I'm a bit tired now

[23:16:38] <brunoais> OK

[23:16:43] <brunoais> tty tomorrow o/

[23:19:39] <temporal_> bye!