3
\$\begingroup\$

Is there anything wrong with using double ternary operator like so:

var stateName:String = state is String ? state as String : "name" in state ? state.name : null;

Here is two lines because someone requested it:

var stateName:String = state is String ? state as String : 
                        "name" in state ? state.name : 
                            null;

Here is the alternative:

if (state is String) {
    stateName = state as String;
}
else if ("name" in state) {
    stateName = state.name;
}
else {
    stateName = null;
}

The function:

/**
 * Sets the style in the state specified. Creates state if it doesn't exist.
 * */
public static function setStyleInState(styleName:String, value:*, state:*, target:UIComponent = null):void {
    var stateName:String = state is String ? state as String : "name" in state ? state.name : null;

    if (!StateUtils.hasState(target, state)) {
        state = createState(stateName, null, target);
    }
    else if (!(state is State)) {
        state = getState(target, stateName);
    }

    // more code to write
}

It makes sense to me but I'm the one writing it.

This is ActionScript3 or ECMAScript 4 (basically JavaScript with strong typing).

\$\endgroup\$
  • 6
    \$\begingroup\$ Except for making your code hard to read? \$\endgroup\$ – Mast Nov 17 '15 at 11:43
  • 1
    \$\begingroup\$ The only thing wrong with your double ternary is that you're writing a double ternary. Less LOC != better readability. Always pretend that you're writing code for an axe murderer who knows where you live. Also, given the javascript comparison, in javascript we never use single line block statements. \$\endgroup\$ – Dan Pantry Nov 17 '15 at 11:43
  • 1
    \$\begingroup\$ It's also a very long line. Is there any reason you can't just make it two lines? \$\endgroup\$ – SuperBiasedMan Nov 17 '15 at 11:44
  • 2
    \$\begingroup\$ This actually seems to be an XY problem. What are you actually trying to achieve? This is almost certainly a code smell. \$\endgroup\$ – Dan Pantry Nov 17 '15 at 11:50
  • 2
    \$\begingroup\$ @1.21gigawatts I much prefer the verbose one. I haven't used AS in a long time but it reads as perfectly clear now what each does. \$\endgroup\$ – SuperBiasedMan Nov 17 '15 at 11:52
5
\$\begingroup\$

Given your example I think that your double ternary operation should actually be a separate function. When you start getting complex one-liners like this, it is a sign that you need another function. Here's how I approached it:

function getStateName(state) {
  if(state is String) {
    return state as String;
  }

  // This will always return "null" if state.name does not exist.
  // I don't know about AS, but in JavaScript if something doesn't
  // exist it returns as undefined (not null), but you want the null
  // return. As far as I am aware, AS supports the || short-circuit and
  // truthy/falsey equivalence, like JavaScript.
  return state.name || null;
}

This could be used in your function like so;

public static function setStyleInState(styleName:String, value:*, state:*, target:UIComponent = null):void {
    var stateName:String = getStateName(state);

    if (!StateUtils.hasState(target, state)) {
        state = createState(stateName, null, target);
    }
    else if (!(state is State)) {
        state = getState(target, stateName);
    }
}

There's more to be said here; I think that exposing mutability like this on a static method (instead of on the instance of a state) is going to lead you into trouble later on down the road and you should probably re-evaluate your design.

Specifically, state in static methods is always a recipe for disaster when it comes to testability and debugging, especially when you branch into multiple threads.

\$\endgroup\$
  • \$\begingroup\$ The original design has all the state functionality on the base class. It became so huge that I thought about using composition over inheritance just to slim it down. I may merge it back in but the base class is already pretty huge. This is my attempt at moving some functionality out of it. \$\endgroup\$ – 1.21 gigawatts Nov 17 '15 at 12:03
  • 1
    \$\begingroup\$ Composition over inheritance isn't a silver bullet and you need to think about it carefully. The idea with composition over inheritance is you have small, self contained components that encapsulate their own state.. static methods destroy that encapsulation. It's a noble goal, but I think you are applying it incorrectly. You could consider posting a separate review and we might be able to help you with that? \$\endgroup\$ – Dan Pantry Nov 17 '15 at 12:04
  • \$\begingroup\$ I don't have any code to review yet. Well I do have the rest of this class. I could try and write up what I plan to do. \$\endgroup\$ – 1.21 gigawatts Nov 17 '15 at 12:14
  • \$\begingroup\$ Here is my initial class. I don't like half the functions but I don't know why. gist.github.com/monkeypunch3/fd11109c88f282adf83d I can post a new question if I need to. \$\endgroup\$ – 1.21 gigawatts Nov 17 '15 at 12:22

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.