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

_______________________________________________
Wikitech-l mailing list
Wikitech-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l

Reply via email to