On Fri, Jan 24, 2014 at 11:19 AM, Nik Everett neverett@wikimedia.orgwrote:

At the architecture summit yesterday we had a conversation about the TitleValue proposal and the vast majority of folks thought it was a great start. Something like 10% of us thought the patch _might_ be a start down the path to Javaify MediaWiki. I was one of the 10%.

We resolved to talk more about the commit before merging it because of the objections of our minority. I felt somewhat vindicated. I slept on it. Now I don't think we made the right choice. I think more discussion is a waste of time and we should just keep moving and try to catch the Javaification if it starts creeping in.

I'm disappointed, I thought you'd be our person to watch for the Javaification.

It looks to me like the existing patch *already is* getting too far into the Javaification, with it's proliferation of classes with single methods that need to be created or passed around.

On 24/01/14 11:19, Nik Everett wrote:

The TitleValue proposal is an improvement over what we have now so I figure we should just do it. Is that ok?

I am also personally in favour of it, but I would like to achieve consensus of the MediaWiki core team if possible before continuing.

On 24/01/14 14:44, Brad Jorsch (Anomie) wrote:

It looks to me like the existing patch *already is* getting too far into the Javaification, with it's proliferation of classes with single methods that need to be created or passed around.

The existing patch is somewhat misleading, since a TODO comment indicates that some of the code in Linker should be moved to HtmlPageLinkRenderer, instead of it just being a wrapper. So that would make the class larger.

-- Tim Starling

Am 24.01.2014 16:15, schrieb Tim Starling:> On 24/01/14 15:11, Jeroen De Dauw wrote:

Daniel proposed an ideal code architecture as consisting of a non-trivial network of trivial classes -- a bold and precise vision. Nobody was uncivil or deprecating in their response.

This idea is something that grew in my mind largely through discussions with Jeroen, and the experience with the code he wrote for Wikidata. My gut feeling for the right balance of locality vs. separation of concerns has shifted a lot though that experience - in the beginning of the Wikidata project, I was a lot more skeptical of the idea of "separation of concerns". Which doesn't mean I insist of going down that road all the way all the time now.

I would like to take this opportunity to thank Jeroen for his conviction and the work he has put into showing that DI and SoC actually make work with a big code base less cumbersome. Without him, we would have copied more problems present in the core code base.

One big advantage I want to highlight is confident refactoring: if you have good, fine grained unit tests, it's easier to make large changes, because you can be confident not to break anything.

Am 24.01.2014 14:44, schrieb Brad Jorsch (Anomie):

It looks to me like the existing patch *already is* getting too far into the Javaification, with it's proliferation of classes with single methods that need to be created or passed around.

There is definitely room for discussion there. Should we have separate interfaces for parsing and formatting, or should both be covered by the same interface? Should we have a Linker interface for generating all kinds of links, or separate interfaces (and/or implementations) for different kinds of links?

I don't have strong feelings about those, I'm happy to discuss the different options. I'm not sure about the right place for that discussion though - the patch? The RFC? This list?

Am 24.01.2014 15:56, schrieb Tim Starling:> On 24/01/14 11:19

The existing patch is somewhat misleading, since a TODO comment indicates that some of the code in Linker should be moved to HtmlPageLinkRenderer, instead of it just being a wrapper. So that would make the class larger.

Indeed. HtmlPageLinkRenderer should take over much of the functionality currently implemented by Linker. The implementation is going to be non-trivial. I left that for later in order to keep the patch concise.

-- daniel

Thanks for your input Nik!

I'll add my 2¢ below. Would be great if others could chime in.

I have just pushed a new version of the path, please have a look at https://gerrit.wikimedia.org/r/#/c/106517/

Am 04.02.2014 16:31, schrieb Nikolas Everett:

• Should linking, parsing, and formatting live outside the Title class?

Yes for a bunch of reasons. At a minimum the Title class is just too large to hold in your head properly. Linking, parsing, and formatting aren't really the worst offenders but they are reasonably easy to start with.

Indeed

I would, though, like to keep some canonical formatting in the new TitleValue. Just a useful __toString that doesn't do anything other than print the contents in a form easy to read.

done

• Should linking, parsing, and formatting all live together in one class

outside the Title class? I've seen parsing and formatting live together before just fine as they really are the inverse of one another. If they are both massively complex then they probably ought not to live together.

There are two questions here: should they be defined in the same interface? And should they be implemented by the same class? Perhaps the answer is no to the former, but yes to the latter...

A good argument for them sharing an implementation is the fact that both formatting and parsing requires the same knowledge: Namespace names and aliases, as well as normalization rules.

Linking feels like a thing that should consume the thing that does formatting. I think putting them together will start to mix metaphors too much.

Indeed

• Should we have a formatter (or linker or parser) for wikitext and another

for html and others as we find new output formats? I'm inclined against this both because it requires tons of tiny classes that can make tracing through the code more difficult

maybe, but I don't think so

and because it implies that each implementation is substitutable for the other at any point when that isn't the case. Replacing the html formatter used in the linker with the wikitext formatter would produce unusable output.

That's a compelling point, I'll try and fix this in the next iteration. Thanks!

I really think that the patch should start modifying the Title object to use the the functionality that it is removing from it. I'm not sure we're ready to start deprecating methods in this patch though.

I agree. I was reluctant to mess with Title just yet, but it makes sense to showcase the migration path and remove redundant code.

In a parallel to getting the consensus to merge a start on TitleValue we need to be talking about what kind of inversion of control we're willing to have. You can't step too far down the services path without some kind of strategy to prevent one service from having to know what its dependencies dependencies are.

Let's try and be clear about how "inversion of control" relates to "dependency injection": you can have IoC without CI (e.g. hooks/listeners, etc), and DI without IoC (direct injection via constructor or setter). In fact, direct DI without IoC is generally preferable, since it is more explicit and easier to test. Specifically, passing in a "kitchen sink" registry object should be avoided, since it makes it hard to know what collaborators a service *actually* needs.

You need IoC only if the construction of a service we need must be deferred for some reason. Prime reasons are

a) performance (lazy construction of part of the object graph)

b) information needed for the construction of the service is only known later (this is really a code small, indicating a design issue - the "service" wasn't really designed as a service).

In any case, yes, we'll need IoC for DI in some cases. In my experience, the best approach usually turns out to be one of the following two:

1) provide a builder function. This is flexible and convenient. The downside is that there is no type hinting/checking, you have to trust that the callback actually implements the expected signature. A single-method factory interface can fix that, but since PHP doesn't have inline classes, these are not as convenient to use.

2) provide a registry that manages the creation and re-use of different instances of a certain kind of sing, e.g. a SerializerRegistry managing serializers for different kinds of things. We may not know in advance what kind of thing we'll need to serialize, so we need to have the registry/factory around. In the simple case, this could be handled via (1) by simply wrapping the registry in a closure, but we may want to access some extra info from the registry, e.g. which serializers are supported, etc.

I don't think we should pick one over the other, just make clear when to use which approach. I can't think of a use case that isn't covered by one of the two, though.

-- daniel

Am 06.02.2014 21:09, schrieb Sumana Harihareswara:

I agree that this mailing list is a reasonable place to discuss the interfaces.

Notes from the Architecture Summit are now up at https://www.mediawiki.org/wiki/Architecture_Summit_2014/TitleValue# . At yesterday's RFC review we agreed that we'd like to hold another one next week (will figure out a good date/time with Nik, Daniel, and the architects) and discuss TitleValue, see if there's anything that needs moving forward.

That would be great, better still if it was during business-hours for me :)

I'm currently working on an alternative approach to the PageLinker and TitleFormatter interfaces, which would result in fewer classes. I'm not sure whether that approach is actually better yet, but since several people have expressed a preference for this, I would like to give it a try. I hope to have this done some time next week.

-- daniel