RL2 Interfaces

[darscan]

http://twitter.com/unclebobmartin/status/131054638840553472

[Stray]

Ok. +1 for that. But... but... but... no. I must keep the faith.

That sounds like a good one-eyed refactor I could do this evening. I will spare you the puns.

What's the convention then for renaming the implementations? Shall I just include something specific about them?

[tschneidereit]

+1 one from me, too.

I like the comments on impl naming in Clean Code: If you can't come up with a specific name for your implementation, that's a good hint for something being wrong with your implementation. To be honest, though, I'm not sure what I'd name the MediatorMap or CommandMap, then.

But: Do we need to have interfaces for everything, then? Why do we need an interface for the CommandMap if the only place it gets created is inside the extension? If we have more than one implementation later on, we can always change things accordingly.

[joelhooks]

I agree with the sentiment, but you'll also see Bob later in the book discuss going along with colloquial naming conventions. My personal stance is that in a framework like this, following the Adobe style guide is the preference even if the I is crufty.

[darscan]

+0.5

[tschneidereit]

While I personally despise the "I" prefix, that's obviously not a good reason for dropping it. And I do see the point about going with the platform's conventions.

On the other hand, it very much seems as though those conventions are slowly shifting.

Plus, I think that, really, the prefix makes interfaces stand out in a way they really shouldn't, making it almost impossible to substitute them for concrete implementations in an API.

[Stray]

My initial reaction to this suggestion was that it was definitely not good to confuse people.

But actually I think the only people it confuses are the ones who are already confused - if that makes sense? Those who have understood the concept will understand that the 'I' doesn't make something special.

And those who haven't understood clearly aren't getting anywhere based on the 'I', so perhaps a new way of teaching IFaces will help?

Also, Shaun's impl/api packages make it quite fast to tell which is which - as soon as you've imported a type you can see where it resides.

That said, we have used the style guide just to avoid the energy / entropy of deliberations on other stuff! I'm happy either way - both have positives.

My number 1 reason to lose the 'I' is that it would be a talking point that would spark a conversation that a lot of developers probably need to have...

[joelhooks]

I think you number 1 reason is interesting. If popular frameworks (Parsley already does this, and it was never an issue) start going this route, it can set the broader course for other devs.

One other mechanical concern is that Flash Builder makes it a pain in the butt to go from interface to implementation. IntelliJ is easy click through.

[Stray]

Maybe I'll try to come up with the names I'd rename the mediator map stuff to.

[Stray]

This tree dump pulls out (with the <-) what needs to be renamed if we drop the 'I'.

<snip/>

See more recent version later in thread.

[darscan]

Just being cheeky, but they don't actually have to change. They are in separate packages.

(likewise for the EventMap btw)

[Stray]

As an aside, simply going through this process led me to recognise that GuardsProcessor and HooksProcessor should implement the same interface.

That makes it a +1.1 from me.

[Stray]

@darscan - very true and I did think of that. But if it even once leads to me having to fully reference a type then I'll be a sad panda. And that's also gonna make browsing the ASDocs Class menu bloody weird!

[darscan]

yep, i wasn't being serious - that would be terrifically annoying!

[darscan]

I like the tree dump btw. though it doesn't look like you've got the latest code.

[Stray]

You're right - I hadn't done any dev today so hadn't pulled yet. I'll pull now and run it again.

[Stray]

+-robotlegs/
  |
  +-v2/
    |
    +-bundles/
    | |
    | +-classic/
    | | |
    | | +-ClassicRobotlegsBundle.as
    | |
    | +-shared/
    |   |
    |   +-configs/
    |   | |
    |   | +-ContextViewWatcherConfig.as
    |   |
    |   +-processors/
    |   | |
    |   | +-ParentContextFinder.as
    |   |
    |   +-utilities/
    |     |
    |     +-LoggingEventDispatcher.as
    |
    +-core/
    | |
    | +-api/
    | | |
    | | +-ContextBuilderEvent.as
    | | |
    | | +-Context.as
    | | |
    | | +-ContextBuilder.as
    | | |
    | | +-ContextBuilderBundle.as
    | | |
    | | +-ContextConfig.as
    | | |
    | | +-ContextExtension.as
    | | |
    | | +-ContextPreProcessor.as
    | | |
    | | +-ContextViewRegistry.as
    | | |
    | | +-TypeFilter.as
    | | |
    | | +-TypeMatcher.as
    | |
    | +-impl/
    | | |
    | | +-Context.as                        <-
    | | |
    | | +-ContextBuilder.as                 <-
    | | |
    | | +-ContextViewRegistry.as            <-
    | | |
    | | +-IsTypeFilter.as                   
    | | |
    | | +-TypeMatcher.as                    <-
    | | |
    | | +-TypeMatcherError.as   
    | |
    | +-utilities/
    |   |
    |   +-classHasMethod.as
    |   |
    |   +-objectHasMethod.as
    |   |
    |   +-pushValuesToClassVector.as
    |
    +-extensions/
    | |
    | +-autoDestroy/
    | | |
    | | +-AutoDestroyExtension.as
    | |
    | +-commandMap/
    | | |
    | | +-api/
    | | | |
    | | | +-CommandMap.as
    | | | |
    | | | +-CommandMapping.as
    | | | |
    | | | +-CommandTrigger.as
    | | |
    | | +-impl/
    | |   |
    | |   +-CommandMap.as
    | |   |
    | |   +-CommandMapping.as
    | |   |
    | |   +-EventCommandTrigger.as
    | |
    | +-guardsandhooks
    | | |
    | | +-api/
    | | | |
    | | | +-LocalProcessor.as
    | | | |
    | | | +-GuardsAndHooksConfig.as
    | | |
    | | +-impl/
    | |   |
    | |   +-GuardsProcessor.as
    | |   |
    | |   +-GuardsAndHooksConfig.as         <-
    | |   |
    | |   +-HookMap.as
    | |   |
    | |   +-HooksProcessor.as
    | |   |
    | |   +-ViewHookMap.as
    | |   |
    | |   +-ViewHookMapExtension.as
    | |
    | +-logging/
    | | |
    | | +-InjectorLoggingExtension.as
    | | |
    | | +-SimpleLoggingExtension.as
    | |
    | +-mediatorMap/
    | | |
    | | +-api/
    | | | |
    | | | +-Mediator.as
    | | | |
    | | | +-MediatorConfig.as
    | | | |
    | | | +-MediatorMap.as
    | | | |
    | | | +-MediatorMapping.as
    | | | |
    | | | +-MediatorTrigger.as
    | | | |
    | | | +-MediatorUnmapping.as
    | | |
    | | +-impl/
    | | | |
    | | | +-FlashMediator.as
    | | | |
    | | | +-FlexMediator.as                     
    | | | |
    | | | +-MediatorConfig.as               <-
    | | | |
    | | | +-MediatorMap.as                  <-
    | | | |
    | | | +-MediatorMapEntry.as             
    | | |
    | | +-utilities/
    | |   |
    | |   +-triggers/
    | |     |
    | |     +-DuckTypedMediatorTrigger.as
    | |     |
    | |     +-RL1MediatorTrigger.as
    | |     |
    | |     +-RL2MediatorTrigger.as
    | |     |
    | |     +-RLMediatorTriggerBase.as
    | |
    | +-viewManager/
    |   |
    |   +-api/
    |   | |
    |   | +-ContainerExistenceEvent.as
    |   | |
    |   | +-ContainerBinding.as
    |   | |
    |   | +-ContainerRegistry.as
    |   | |
    |   | +-ViewClassInfo.as
    |   | |
    |   | +-ViewHandler.as
    |   | |
    |   | +-ViewManager.as
    |   | |
    |   | +-ViewProcessor.as
    |   | |
    |   | +-ViewWatcher.as
    |   | |
    |   | +-ViewHandlerEvent.as
    |   |
    |   +-AutoStageWatcherExtension.as
    |   |
    |   +-ConfigureViewWatcherExtension.as
    |   |
    |   +-impl/
    |   | |
    |   | +-ContainerBinding.as             <-
    |   | |
    |   | +-ContainerRegistry.as            <-
    |   | |
    |   | +-ViewClassInfo.as                <-
    |   | |
    |   | +-ViewManager.as                  <-
    |   | |
    |   | +-ViewProcessor.as                <-
    |   |
    |   +-utilities/
    |   | |
    |   | +-watchers/
    |   |   |
    |   |   +-AutoStageWatcher.as
    |   |   |
    |   |   +-ConfigureViewEvent.as
    |   |   |
    |   |   +-ConfigureViewWatcher.as
    |   |
    |   +-ViewManagerExtension.as
    |
    +-flex/
      |
      +-mxml/
        |
        +-ContextBuilderTag.as

[joelhooks]

I genuinely dislike having the interface and implementation with the same name. Impl is better off as DefaultImpl imo. Sure, they are separated by a package, but it is really inconvenient from a code hinting perspective.

On Monday, October 31, 2011 at 3:42 PM, Stray wrote:

+-robotlegs/	+-v2/

+-bundles/			+-classic/
	+-ClassicRobotlegsBundle.as
+-shared/
+-configs/
	+-ContextViewWatcherConfig.as
+-processors/
	+-ParentContextFinder.as
+-utilities/
+-LoggingEventDispatcher.as

+-core/			+-api/
	+-ContextBuilderEvent.as
	+-Context.as
	+-ContextBuilder.as
	+-ContextBuilderBundle.as
	+-ContextConfig.as
	+-ContextExtension.as
	+-ContextPreProcessor.as
	+-ContextViewRegistry.as
	+-TypeFilter.as
	+-TypeMatcher.as
+-impl/
	+-Context.as <-
	+-ContextBuilder.as <-
	+-ContextViewRegistry.as <-
	+-IsTypeFilter.as <-
	+-TypeMatcher.as <-
	+-TypeMatcherError.as
+-utilities/
+-classHasMethod.as
+-objectHasMethod.as
+-pushValuesToClassVector.as

+-extensions/			+-autoDestroy/
	+-AutoDestroyExtension.as
+-commandMap/
	+-api/
		+-CommandMap.as
		+-CommandMapping.as
		+-CommandTrigger.as
	+-impl/
	+-CommandMap.as
	+-CommandMapping.as
	+-EventCommandTrigger.as
+-guardsandhooks
	+-api/
	+-LocalProcessor.as
	+-GuardsAndHooksConfig.as
	+-impl/
	+-GuardsProcessor.as
	+-GuardsAndHooksConfig.as <-
	+-HookMap.as
	+-HooksProcessor.as
	+-ViewHookMap.as
	+-ViewHookMapExtension.as
+-logging/
	+-InjectorLoggingExtension.as
	+-SimpleLoggingExtension.as
+-mediatorMap/
	+-api/
		+-Mediator.as
		+-MediatorConfig.as
		+-MediatorMap.as
		+-MediatorMapping.as
		+-MediatorTrigger.as
		+-MediatorUnmapping.as
	+-impl/
		+-Mediator.as <-
		+-MediatorConfig.as <-
		+-MediatorMap.as <-
		+-MediatorMapEntry.as
	+-utilities/
	+-triggers/
	+-DuckTypedMediatorTrigger.as
	+-RL1MediatorTrigger.as
	+-RL2MediatorTrigger.as
	+-RLMediatorTriggerBase.as
+-viewManager/
+-api/
	+-ContainerExistenceEvent.as
	+-ContainerBinding.as
	+-ContainerRegistry.as
	+-ViewClassInfo.as
	+-ViewHandler.as
	+-ViewManager.as
	+-ViewProcessor.as
	+-ViewWatcher.as
	+-ViewHandlerEvent.as
+-AutoStageWatcherExtension.as
+-ConfigureViewWatcherExtension.as
+-impl/
	+-ContainerBinding.as <-
	+-ContainerRegistry.as <-
	+-ViewClassInfo.as <-
	+-ViewManager.as <-
	+-ViewProcessor.as <-
+-utilities/
	+-watchers/
	+-AutoStageWatcher.as
	+-ConfigureViewEvent.as
	+-ConfigureViewWatcher.as
+-ViewManagerExtension.as

+-flex/	+-mxml/

+-ContextBuilderTag.as

Reply to this email directly or view it on GitHub: https://github.com/robotlegs/robotlegs-framework/issues/43#issuecomment-2582944

[joelhooks]

I see what you've done ;P

(missed the <-)

[Stray]

Yes - the <- are the ones that need renaming.

Still - I think I already got 2 wins from the process. One unified interface, one divergent implementation: Flash vs FlexMediator - we shouldn't be hiding (from the user's brain) the implementation difference of waiting for creationComplete on UIComponent extenders.

Or I might be wrong. It does get annoying when you want a SignalMediator. Ho hum. Maybe just EventDrivenMediator is better?

[darscan]

Issue for mediator map api & impl:

https://github.com/robotlegs/robotlegs-framework/issues/30

[fljot]

I vote for I.

In personal (and public) perception — it's common and used, clean, not-confusing (as opposite to new brave styles). In specific code — it allows you to have "same" names for default implementation (which is the regular case in RL) without any pain. In platform subject — it's a standard for the AS3 even at the native classes level (not to speek of the sole adobe flex coding conventions).

[weyert]

I would vote for I too. Adding Impl is too much Java for me. I guess I can live with the impl namespace, though. Just never understand why you want to remove I and then postfix classes with Impl instead...

[joelhooks]

I agree about the AS3 base classes, but apps are largely away from these core interfaces where RL is in play.

Code normal. It is more friendly to others.

But I hate the prefix.

On Oct 31, 2011, at 17:25, Pavel Pevnitskiyreply@reply.github.com wrote:

I vote for I.

In personal (and public) perception — it's common and used, clean, not-confusing (as opposite to new brave styles). In specific code — it allows you to have "same" names for default implementation (which is the regular case in RL) without any pain. In platform subject — it's a standard for the AS3 even at the native classes level (not to speek of the sole adobe flex coding conventions).

Reply to this email directly or view it on GitHub: https://github.com/robotlegs/robotlegs-framework/issues/43#issuecomment-2584250

[neilmanuell]

I would vote for removing the I The only reason I continue to use it is convention. If the reason most people use it is convention (because they disagree and use the standard, or that's the way they were taught) that's a bit silly. My day to day dislike focuses on the alphabetical ordering in a package, which of course is removed in your package structure (which I like a lot more than the rather ambiguous core / base ).

[creynders]

I lean towards keeping the I. TBH I don't understand why it's disliked so much. If you'd be able to fully swap an interface and a class it would be something different all together, but in AS that's not the case. And I find it extremely annoying not to be able to tell them apart when reading through API docs.

And having a class and an interface with the same name just makes me shudder, that's a horrible idea, but I think we all agree on that.

[tschneidereit]

Why can't you fully swap them out? For me, one of the main points about dropping the prefix is to allow exactly that. The only place where you have to worry about whether something is a class or an interface is during construction, but that is nicely dealt with using an IoC container.

And once you're able to treat interfaces and classes the same otherwise, you don't have to introduce redundant interfaces for types that have only one implementation, removing the problem of same-named classes and interfaces.

On Tue, Nov 1, 2011 at 14:42, creynders reply@reply.github.com wrote:

I lean towards keeping the I. TBH I don't understand why it's disliked so much. If you'd be able to fully swap an interface and a class it would be something different all together, but in AS that's not the case. And I find it extremely annoying not to be able to tell them apart when reading through API docs.

And having a class and an interface with the same name just makes me shudder, that's a horrible idea, but I think we all agree on that.

Reply to this email directly or view it on GitHub: https://github.com/robotlegs/robotlegs-framework/issues/43#issuecomment-2590301

[creynders]

That's what I meant with not being able to fully swap them: you need to know the difference during construction. But I don't get what you mean with "once you're able to treat interfaces and classes the same otherwise, you don't have to introduce redundant interfaces for types that have only one implementation" What's the difference? Why would you introduce redundant interfaces for types that have only one implementation when prefixing with I? I mean the whole point of having interfaces IS to be able to swap the implementation, whether you prefix them or not. To me the main point still remains that an interface is a contract, an intent, a concept, while an implementation is concrete, they are separate things and therefore a naming difference is totally understandable. I guess I just don't see the benefits of NOT prefixing them, while I do see a number of benefits when you do. But I know a lot of far more experienced and smarter people than me hate the I prefixes, I just don't seem to grasp why.

[Stray]

@creynders - Till's point is that if your class is the implementation then a redundant interface isn't useful. And without the I, at the point at which you expand the selection of implementations, you can rename the original class to explain how it implements X, make the original class name the interface, and away you go.

Where I've the 'I' really useful though is when I'm doing modular development and want to be sure I'm not pulling concrete classes in... but as FlexPMD lets you use api/restricted to guard against that anyway, I'm not sure I'm getting mileage from that anymore.

When you're using Robotlegs / Swiftsuspenders, mostly you're not using constructors - certainly not for the framework classes anyway. As Till says, if you [Inject] a MediatorMap it shouldn't matter to you whether MediatorMap is being implemented by a class of the same name or a class with a different name - as long as it satisfies that type.

I was really reluctant about this, but just in playing with it yesterday briefly I got some new insights into my implementation, so that's won me over. My big remaining hesitation is that it makes code review tougher - right now I can whip people pretty easily because I can see that they're returning an implementation and not an interface, and with this approach I'd need to ask - or at least scroll up the file and see whether it's in the api or impl / restricted folder.

Sx

[Stray]

[Insert comment from Till about how if I used a real IDE for code review I'd be able to see interfaces / concrete classes in different colours or something... ;) ]

[tschneidereit]

Thanks Stray: Your explanation is exactly what my rambling didn't accomplish to bring across at all.

And no: No snarkiness about IDEs on this one. But a small gripe about how in a code review it shouldn't matter what kind of type is returned. It really only matters because people do too much unmanaged construction of classes - and because of the "I" prefix. If something returns a type, why does it matter what kind of type it is?

[Stray]

It doesn't always matter, but sometimes it does - especially if the intent of the code is to provide something that is going to be used by modules.

A good example is custom events: often I want to pass them around from module to module and I don't want the actual construction of a VO to be compiled into all the modules, I just want the interface, so that I can refactor under the hood.

[tschneidereit]

Ok, fair point - and sorry for not picking up on your earlier mention of that particular problem.

Sounds like a good use-case for FlexPMD or tools in general, though.

[Stray]

Yes - it definitely is, but when you're just doing informal 'looking over', it's the kind of thing that is useful to see.

Still - you can check it by checking the package and the tools do a good job too.

[honi]

Maybe the issue is that the framework only provides one Context, one MediatorMap, one CommandMap, etc out of the box. You don't have a second Context to force you to find two unique names.

The idea of having an interface and coding against that interface, not caring of the concrete implementation, is that one could swap that implementation very easily, right? I bet if I would roll up my own Context for whatever reason, I won't name it just Context, it would have a more meaningful name.

[joelhooks]

This is actually part of the point. A Context is the interface, any implementation of that interface will be named appropriately. There is no (should not be?) an implementation as generic as to be named Context. That name goes to the contract.

Thats the idea anyway.

On Nov 1, 2011, at 18:57, Joni Bekensteinreply@reply.github.com wrote:

Maybe the issue is that the framework only provides one Context, one MediatorMap, one CommandMap, etc out of the box. You don't have a second Context to force you to find two unique names.

The idea of having an interface and coding against that interface, not caring of the concrete implementation, is that one could swap that implementation very easily, right? I bet if I would roll up my own Context for whatever reason, I won't name it just Context, it would have a more meaningful name.

Reply to this email directly or view it on GitHub: https://github.com/robotlegs/robotlegs-framework/issues/43#issuecomment-2597904

[honi]

I agree. The interface should be named Context. But how would you name the built in concrete Context? SimpleContext?

[joelhooks]

Standard is something like DefaultContext. You'd name it something targeted and awesome! That's the hard part, really.

On Nov 1, 2011, at 20:03, Joni Bekensteinreply@reply.github.com wrote:

I agree. The interface should be named Context. But how would you name the built in concrete Context? SimpleContext?

Reply to this email directly or view it on GitHub: https://github.com/robotlegs/robotlegs-framework/issues/43#issuecomment-2598417

[creynders]

@stray thanks for the explanation, I think I'm finally starting to get it. Just to be sure, could you check the small gist I made to see if I understood it correctly? https://gist.github.com/5c8c418c587ea4ecff0c

Sorry for my thickness!

[Stray]

@creyenders - exactly!

Not thick at all, I think it has taken about six months for it to click in my head since Till first mentioned it.

[weyert]

When do you guys plan to remove the I then I can mentally prepare myself the next few weeks...

[neilmanuell]

@creynders nice explanation. Good to get something solid to point at. BTW I've just re-read this post in its entirety... it makes a very interesting read

[Stray]

I have one request: if we lose the 'i' then can we also have all lowercase package names?

After all, the justification for camelCase packages was that it's in the style guide. Oddly, FlexPMD ships with "A package name should be lower case (org.robotlegs.v2.extensions.viewManager). Detects when a package definition contains upper case characters."

Then, in years to come, people will ask "Why doesn't Robotlegs use I for interfaces?" and the answer will be "Because they didn't want to have to use camelCase for package names... ".

Adobe don't even apply the camelCase consistently themselves... fl.controls.listClasses and fl.livepreview for example.

Why do they make our lives so hard? sigh ;)

[Stray]

A more serious suggestion - we could refactor to this relatively soon, and then we have plenty of time for people to "Suck it and see." as we say in the UK.

The change could, at this point, be underpinned by the possibility of going back to using the I, with a promise to have a further discussion just before we reach the first alpha release candidate?

[creynders]

@stray thanks for handing me the key to finally grasp what it's all about...

Funny thing is I'm actually even more convinced that keeping the I is a good idea for several reasons: 1/ to me the implement-first-refactor-to-interface-later is actually bad practice, because it encourages people to not think in interfaces. I really NEED to start with an interface because it forces me to think deep about the API. If I don't, I'll definitely end up with using setter methods at places where I shouldn't, for example. 2/ Most people won't refactor an implementation to an interface, find out where they were constructing that implementation and swap it to the new one. Why? Due to the situation in which you first realize that you need to switch from impl to iface: you want to SWAP the impl. The cases where you'll want to ADD an impl will be rare. So I think most people will just start to refactor the impl itself and not refactor it to an iface. So I think we really have to ask ourselves whether we want to be encouraging coding to an impl instead of an iface? 3/ What with RL? The above situation will NEVER occur. I can't imagine you'll be adding concrete implementations to the api package in order to refactor them later, so you'll end up creating interfaces anyway.

So if it actually IS bad practice I'd do my best to let people avoid that and force them to realize they're using an iface and not an impl, hence the prefix. And it's a platform standard too of course. Not that I think that's a very convincing argument: I strongly believe it's better to change standards than to confirm to bad ones.

[Stray]

I agree with all the above, and they're exactly why I was hesitant...

In the end I realised that the way we currently do it doesn't work either. People who take shortcuts take shortcuts, people who write clean code write clean code. There are a percentage of devs who feel they have good OO understanding but actually don't around this area... we're never going to change their practice by pulling on the same levers that already aren't working.

That's my justification - anyone who 'gets it' will continue to get it, and anyone who doesn't has had plenty of time with the IFace protocol - maybe a new angle / challenge will help them to see it from a new angle.

Maybe?

[neilmanuell]

I think that if the aim is to protect people from their own mistakes they will never learn (and I'm including me in that of course). The only other way is to let them make their own mistakes, in which case they will either blame you, and move on to something else, or listen and learn ( but only if there are people patient and willing enough to give time and effort into it).

[creynders]

@neilmanuell, @stray: agreed, and I don't think we should necessarily be educators per se, but there's a difference between letting people take shortcuts and encouraging them to take shortcuts. That's why I like RL so much, it encourages best practices but gives me leeway enough to take shortcuts consciously whenever I see fit.

And regarding the packagenames : I'm definitely in favor of all lowercase.

[creynders]

One last thing: I think ISomeClass with SomeClass as implementation is also preferable over SomeClass with DefaultSomeClass for two reasons: 1/ DefaultSomeClass says to me several implementations are available already and this one is simply the out-of-the-box one, while SomeClass implementing ISomeClass tells me this is the default and possibly only implementation 2/ Since class sorting in the IDE was brought up: imagine having DefaultSomeClass, DefaultAnotherClass, DefaultWhateverClass,... Granted, it won't happen a lot due to package names, but still.

And talking about IDE's: one of the reasons why it would be better to have non-prefixed ifaces is to be able to refactor a concrete implementation into an iface, w/o refactoring the clients. But with most IDE's renaming a class is a piece of cake, so to me that's not a very convincing argument either.

[neilmanuell]

@creynders re sorting. True, which is why I would post fix: ContextDefault, which will probably illicit a few shudders, and arguments too.

[creynders]

@neilmanuell shudder that's just ... painful even ;)