RL2 Guards and Hooks

[darscan]

An issue to discuss the Guards and Hooks API and implementation.

[darscan]

I wonder if it would be useful to be able to provide extra parameters to the guards/hooks.

see: http://code.google.com/p/fabrication/wiki/Interceptors

Also, the add(...guardClasses) signature seems to be more trouble than it's worth (checking for a single element array, and then checking if that element is itself an array/vector, etc). I'd like to drop pushValuesToClassVector utility. It all just feels a bit messy at the moment.

What about:

add(guardClass:Class, parameters:Object=null);

which would then be used like so:

guards.add(SomeGuard, {strict:true});

or, when using the command map:

commandMap.map(SomeCommand).toEvent("someEvent").withGuard(BossGuard, {fallbackResponse:true});

[neilmanuell]

Continuing question about use case for Command Hooks. I can't think of one that is not a bit dirty. Most things can be covered by automated injection or Guards, can't they?

[Stray]

@darscan - The pushValuesToClassVector method is used all over the place.

So we'd also need to drop that api style for type matchers and so on because it's used everywhere. I don't mind forcing people to pass an array - but we'll still need to do some checking because it should be valid to pass a class vector as well, so this is the only way to support all of those possibilities.

Not sure on the parameters - I can't think of a usecase that isn't better achieved by having an injected model that determines the default behaviour. It feels like putting logic in configuration to me. Not sure though...

@neilmanuell - A use case for hooks is a calculation that needs to be done over multiple commands, but requires injections itself - to avoid the command having a dependency on all the bit part players, when it just requires the output. For example in a situation where you were checking an account's outstanding liabilities (as yet unpaid) before deciding whether a transaction was viable.

[darscan]

@neilmanuell Guards run before the command is instantiated, whereas hooks run after instantiation but before execution. Guards should simply say yes/no, whereas hooks are free to modify the command instance itself.

@Stray Yeh, I know that those utilities are used elsewhere. I still feel like a utility is a code smell indicating broken encapsulation (or some other design problem). Let's keep them for now and see if they can be refactored away later.

[Stray]

@darscan In this case the utility is a code smell that Adobe's implementation of Vectors sucks.

[darscan]

:)

Although, that utility has little to do with the vector implementation and is more about dealing with ...rest params and the side effects of using rest params in an API.

[Stray]

We would still need a version of it for giving the option to pass an array or a vector. At least that would seem saner to me than duplicating the same function in every class that uses it.

Do you not have a bunch of utility functions that you use generally for working with Vector/Array ambiguity? To me they feel no different to the trimWhitespace() kind of String functions that I use over and over.

Obviously we could have put it in a VectorUtils static class... would that feel different?

The problem as I see it is that to preserve AS3 backward integrity, Adobe didn't give Array and Vectora shared base / interface that would allow us to specify ArrayOrVector as a type. So you have to type to :Object or :*, at which point folk might pass in the individual item, and only giving a runtime error for that situation feels a bit tight when it's fairly easy to handle.

So not having ...rest doesn't get rid of the problem unless we're prepared to require people to pass a Vector.<Class>, or we decide to give up the benefits that Vector gives us internally, or we return to using concat() and not making internal holders const.

Or we make people specify classes individually, which for the TypeMatcher would be horrible...!

[neilmanuell]

@darscan I know Guards of old :)

@Stray I see - cool. I used to create Command helpers, but never knew where to put them. Hooks are a nice formalisation thanks for the clarification... I need examples to get the picture.

[darscan]

All fair points. It's a tricky situation. A VectorUtils class would feel worse actually :)

unless we're prepared to require people to pass a Vector.

Nooooo!

However, we are looking at the problem from two different angels. I absolutely see it as a problem of dealing with ...rest signatures, whereas I think you're looking more at the concatenation problem. These are two separate concerns, and the utility currently deals with both. I wonder if the utility couldn't be more general.

flattenParams(array):Array

This would do the work of checking for single element arrays where that element is itself an array/vector. It would return a flattened array.

Any API that exposes ...rest params would use this to ensure that the params are flat, but would handle the pushing/splicing of the elements internally (whose storage format is an implementation detail and should not be exposed).

function withGuards(...guardClasses)
{
  guardClasses = flattenParams(guardClasses);
  for each (var guardClass:Class in guardClasses)
  {
    // implementation detail. Might push to vector, array or use a dictionary, not important
  }
}

[Stray]

Of course it could be more generic, but then you lose the benefits of using Vectors, which is why I tend to make them more specific for Vector work, and also you're still duplicating code in the most common case in our situation, which is putting things in a Vector of Classes.

How do you normally work with Vectors yourself if not using this kind of utility?

We're not exposing the storage format as I see it - the function is used internally, and is stateless, so there's no exposure in the current set up either. No more exposure than using Math.round() for example as far as I can tell.

Yes, wanting a vector of classes is an implementation detail, and this is a utility for helping that implementation, it doesn't expose it anywhere... does it?

I'm not saying flattenParams isn't also useful, but I really don't understand what's exposing about the current setup... what am I missing? Is there something fundamentally missing in my understanding of what is / isn't exposure? (There could be...!)

[darscan]

No, you're right, if those utilities are used internally then nothing is exposed. Ok, let's stick with this for a while. At least until the Yagninator descends upon the codebase :)

[darscan]

Guards and hooks are pretty much sorted now I think. Going to close this, we can open it up again if need be.