1
\$\begingroup\$

This question is regarding my approach to the following implementation requirement. Can you suggest a better alternative that doesn't suffer from the issues I'll point out in my implementation?

REQUIREMENT:

Create an input field (View) that updates itself automatically whenever a specific property on the Model changes.

  • The input field should be generic (ie: it should work for any Model, and any appropriately typed property)
  • The Parent View that holds the Field should not be responsible for updating the field when the Model changes
  • The programming language is ActionScript, which is a strongly typed EMCAScript variant. I'm not sure whether this is relevant, since this question doesn't deal with code directly.

MY SOLUTION:

Model implements an Interface that defines the following methods:

  • registerInput(): the Field registers itself with the model and indicates which properties it wants to monitor
  • unregisterInput(): for cleanup /gc purposes
  • updateInputField(): the Field can update the model when the user interacts with it

Input Field implements an Interface that defines the following method:

  • updateField(): the Model can call this on any registered Field, when a monitored property changes

The Input Field is instantiated by the Parent View (that has a reference to the Model), by providing it with the Model instance (typed to the Interface) and the NAME (String) of the property on the Model we're want to monitor.

ISSUES:

This solution works well, but there are two issues, both dealing with the fact that the Model's property is passed as a string, which means the IDE can't use autosuggest or type checking.

  1. The Parent View passes the Model property as a string. So if there's a typo, we'll only find out about it at runtime, and we have to copy-and-paste it from the Model's function definition.

  2. The Model updates the Field by passing the property as a string, with similar drawbacks.

  3. The value of the property can't be typed for the IDE.

Code:

ILiveModel

public interface ILiveModel 
{
    function registerLiveListener(liveListener:ILiveListener, fields:Array):Array;
    function unregisterLiveListener(liveListener:ILiveListener):void ;
    function updateLiveField(fieldName:String, newValue:*):void;
    function getLiveFieldValue(fieldName:String):*;
}

ILiveListener

public interface ILiveListener 
{
    function updateLiveField(field:String, value:*):void;
}

ILiveModel implementation

public class KioskModel extends EventDispatcher implements ILiveModel {
    private var _termsAndConditionsAgree:Boolean = false;
    protected var _liveListeners:Dictionary = new Dictionary(true);
    protected var _monitoredFields:Object = { };

    public function get termsAndConditionsAgree():Boolean {
        return _termsAndConditionsAgree;
    }

    public function set termsAndConditionsAgree(agree:Boolean):void {
        _termsAndConditionsAgree = agree;

        updateRegisteredLiveListeners("termsAndConditionsAgree");
    }

    public function registerLiveListener(liveListener:ILiveListener, fields:Array):Array {
        var monitoredFieldListeners:Array;
        var fieldValues:Array = [];

        trace("<o> Registering ", liveListener, " to watch ", fields);

        // Register the input in the Dictionary, so we can unregister its fields when asked to
        _liveListeners[liveListener] = fields;

        // For each field we've been asked to monitor, add the input, unless it's already watching that field
        for each(var field:String in fields) {
            if (! field in this) {
                throw new ArgumentError("Cannot register Listener Field '" + field + "' on model " + this + ": property or method does not exist");
            }
            monitoredFieldListeners = _monitoredFields[field];

            if (monitoredFieldListeners && monitoredFieldListeners.length) {
                if (monitoredFieldListeners.indexOf(liveListener == -1)) {
                    monitoredFieldListeners.push(liveListener);
                    _monitoredFields[field] = monitoredFieldListeners;
                }
            } else {
                _monitoredFields[field] = [liveListener];
            }

            fieldValues.push(getLiveFieldValue(field));
        }

        // Return an array of all the values we're watching, for convenience.
        return fieldValues;
    }

    public function unregisterLiveListener(liveListener:ILiveListener):void {
        var monitoredFields:Array = _liveListeners[liveListener];
        var monitoredFieldListeners:Array;
        var fieldIndex:int;

        trace("<-> Unregistering ", liveListener);

        if (monitoredFields) {
            for each (var field:String in monitoredFields) {
                monitoredFieldListeners = _monitoredFields[field];

                if (monitoredFieldListeners && monitoredFieldListeners.length) {
                    fieldIndex = monitoredFieldListeners.indexOf(liveListener);

                    if (fieldIndex != -1) {
                        monitoredFieldListeners.splice(fieldIndex, 1);

                        if (monitoredFieldListeners.length){
                            _monitoredFields[field] = monitoredFieldListeners;
                        } else {
                            delete _monitoredFields[field];
                        }
                    }
                }
            }
        }

        delete _liveListeners[liveListener];
    }

    public function updateLiveField(fieldName:String, newValue:*):void {
        if (fieldName in this) {
            this[fieldName] = newValue;
        } else {
            throw new ArgumentError("Cannot update Listener Field '" + fieldName + "' on model " + this + ": property or method is not valid.");
        }
    }

    public function getLiveFieldValue(fieldName:String):* {
        if (fieldName in this) {
            return (this[fieldName]);
        } else {
            throw new ArgumentError("Listener Field '" + fieldName + "' in " + this + " is not a valid property or method");
        }
    }

    protected function updateRegisteredLiveListeners(fieldName:String) {            
        var liveListeners:Array = _monitoredFields[fieldName];

        if (liveListeners) {
            for each (var liveListener:ILiveListener in liveListeners) {
                liveListener.updateLiveField(fieldName, this[fieldName]);
            }
        }
    }
}

ILiveListener implementation

public class InputRadioButtonView extends AssetView implements ILiveListener {
    private const LABEL_OFF = 'off';
    private const LABEL_ON = 'on';

    protected var _model:ILiveModel;
    protected var _modelProperty:String;
    protected var _value:Boolean;

    public function InputRadioButtonView(assetClip:*, model:ILiveModel, modelProperty:String, disabled:Boolean=false, localizableTextFieldName:String="label") {
        super(assetClip, disabled, localizableTextFieldName);

        _model = model;
        _modelProperty = modelProperty;
    }

    public function updateLiveField(field:String, value:*):void {
        updateClip();
    }


    /**
     * Be sure the clip accurately reflects the current value, every time we switch to its screen.
     */
    override public function setupBeforeLocalize():void {
        updateClip();

        super.setupBeforeLocalize();
    }

    override public function activate() {
        var values:Array;

        if (activatable) {
            clip.addEventListener(MouseEvent.CLICK, onClick, false, 0, true);
            values = _model.registerLiveListener(this, [_modelProperty]);

            DebugUtils.print_r(values);
        }

        super.activate();
    }

    override public function deactivate() {
        if (deactivatable) {
            clip.removeEventListener(MouseEvent.CLICK, onClick);
            _model.unregisterLiveListener(this);
        }

        super.deactivate();
    }

    /**
     * Attempts to update the clip, and lets you know if it was successful or not.
     * @return
     */
    protected function updateClip():Boolean {
        // Stash the value for convenience
        _value = _model.getLiveFieldValue(_modelProperty);

        if (clip){
            if (_value) {
                movieClip.gotoAndStop(LABEL_ON);
            } else {
                movieClip.gotoAndStop(LABEL_OFF);
            }

            return true;
        }

        return false;
    }

    protected function onClick(event:MouseEvent):void {
        _model.updateLiveField(_modelProperty, ! _value);
    }
}

Parent View implementation

public class HowThisWorksBox extends ContentAssetView {
    private var _kioskModel:KioskModel;

    public function HowThisWorksBox(clip:*, disabled:Boolean = false) {
        super(clip, ["heading", "body"], disabled);
    }

    override protected function defineFlexItems(parentItem:FlexItem = null):FlexItem {
        _kioskModel = KioskModel.getInstance();

        return super.defineFlexItems(parentItem)
            .chainAsset(new ButtonAssetView("terms_button", ["label"], onTermsButtonClicked))
            .chainAsset(new InputRadioButtonView("agree_button", _kioskModel, "termsAndConditionsAgree"));
    }


    private function onTermsButtonClicked():void {
        screen.router.setScreen("main__terms");
    }
}

Leverages the Kestrel mobile development framework - which defines the superclasses of this View.

\$\endgroup\$
3
\$\begingroup\$

Disclaimer: Absolutly no knowledge about actionscript.


InputRadioButtonView

protected function updateClip():Boolean {
    // Stash the value for convenience
    _value = _model.getLiveFieldValue(_modelProperty);

    if (clip){
        if (_value) {
            movieClip.gotoAndStop(LABEL_ON);
        } else {
            movieClip.gotoAndStop(LABEL_OFF);
        }

        return true;
    }

    return false;
}  

If I see a construct like this, the little guy in my head screems USE A GUARD CLAUSE to safe vertical spacing.

Another voice will ask me "Why is this _value variable filled if clip != true?"

The next one just states, "I can't read the code that easy because there aren't always spaces before a brace {.

So fixing these three points to silence the voices in my head is pretty straightforward like so

protected function updateClip():Boolean {

    if (!clip) {
        return false;
    }

    // Stash the value for convenience
    _value = _model.getLiveFieldValue(_modelProperty);

    if (_value) {
        movieClip.gotoAndStop(LABEL_ON);
    } else {
        movieClip.gotoAndStop(LABEL_OFF);
    }

    return true;

}  

KioskModel

public function unregisterLiveListener(liveListener:ILiveListener):void {
    var monitoredFields:Array = _liveListeners[liveListener];
    var monitoredFieldListeners:Array;
    var fieldIndex:int;

    trace("<-> Unregistering ", liveListener);

    if (monitoredFields) {
        for each (var field:String in monitoredFields) {
            monitoredFieldListeners = _monitoredFields[field];

            if (monitoredFieldListeners && monitoredFieldListeners.length) {
                fieldIndex = monitoredFieldListeners.indexOf(liveListener);

                if (fieldIndex != -1) {
                    monitoredFieldListeners.splice(fieldIndex, 1);

                    if (monitoredFieldListeners.length){
                        _monitoredFields[field] = monitoredFieldListeners;
                    } else {
                        delete _monitoredFields[field];
                    }
                }
            }
        }
    }

    delete _liveListeners[liveListener];
}  

This almost looks like a example of the arrow code antipattern which could be avoided by using a guard clause and continue; like so

public function unregisterLiveListener(liveListener:ILiveListener):void {
    var monitoredFields:Array = _liveListeners[liveListener];
    var monitoredFieldListeners:Array;
    var fieldIndex:int;

    trace("<-> Unregistering ", liveListener);

    if(!monitoredFields) {
        delete _liveListeners[liveListener];
        return; // don't know if this is possible in action script
    }

    for each (var field:String in monitoredFields) {
        monitoredFieldListeners = _monitoredFields[field];

        if (!(monitoredFieldListeners && monitoredFieldListeners.length)) {
            continue;
        }

        fieldIndex = monitoredFieldListeners.indexOf(liveListener);

        if (fieldIndex != -1) {
            monitoredFieldListeners.splice(fieldIndex, 1);

            if (monitoredFieldListeners.length){
                _monitoredFields[field] = monitoredFieldListeners;
            } else {
                delete _monitoredFields[field];
            }
        }
    }

    delete _liveListeners[liveListener];

}  
\$\endgroup\$
  • \$\begingroup\$ Thanks for the good advice: I learned a few things here! \$\endgroup\$ – Tom Auger Aug 14 '15 at 10:07

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.