Script.Value does not do what it says it does

[dlev-]

Script.Value gets translated to || in js. || does a truthyness test on values, but the Script.Value documentation says "Gets the first valid (non-null, non-undefined, non-empty) value."
Here's example code;

bool foo = false;
bool bar = Script.Value(foo, true);

Since foo is not null or undefined, I would expect bar to be false, but as Script.Value is currently translated it will be true. Based on the name of the function, and the documentation, I would expect Script.Value to return the value of the first argument that passes the test of Script.IsValue

[nikhilk]

This is a good catch.

You make a good argument about having shared semantics between IsValue and Value. Yet, at the same time, I'll accept the documentation is actually incorrect, because the goal of Script.Value was to provide the ability to generate the common script pattern of doing an || to get the first truthy value (the reason being in c# || is only supported on booleans, and here you want it to work on any types).

I definitely want to provide the ability to do an || to find the first truthy value with a compact syntax that one would use when manually writing script. So I wouldn't want to change how Script.Value works. Perhaps it should be renamed to not be as closely associated with IsValue.

I imagine ideally this is what we could have instead:

Script.IsValue  -> conceptually equivalent to !IsNullOrUndefined (stays as-is)
Script.IsTruthy -> addition; returns true for non-null, non-undefined,
                             non-zero, non-empty string and true
Script.Truthy   -> replaces Script.Value; returns first value matching IsTruthy
                                          via the || syntax

Makes sense?

Obviously its a breaking change ... any others care to comment?

[jrockwood]

So, would Script.IsTruthy(x) simply compile down to x? For example,

// Script#
if (Script.IsTruthy(x)) {
    // do something
}

// Compiled JavaScript
if (x) {
    // do something
}

I would suggest having a Script.IsFalsey(x) that compiles down to !x also.

I like renaming Script.Value to Script.Truthy, even though it's a breaking change. I would suggest Script.FirstTruthy, though to make it clearer.

[nikhilk]

The script for IsTruthy will actually be !!(x) ... since the expression might be a variable assignment/parameter value rather than in an if statement, so it would be good to end with an actual boolean value. Good suggestion on IsFalsey. And yes, it would simply generated !(x). Note the parens in both cases. I haven't tried it but maybe the minimizer does some contextual minimization to optimize the few extra characters in the case of an if statement.

I see FirstTruthy as being clearer... yet having some subconscious dislike. Maybe if I stare at that name long enough I'll get over it. :)

I think it has to do with being a multi-part name without a verb in front. For example, SelectFirstTruthy sounds better, but on the flip-side looks quite long.

[nikhilk]

As I made the change, I am not sure I am happy about either name - Truthy or FirstTruthy. For example, here is a use-case:

httpServer.Listen(Script.Truthy(Node.Process.Environment["port"], 8888))
httpServer.Listen(Script.FirstTruthy(Node.Process.Environment["port"], 8888))

I don't think either communicates the intent well. I think "truthy" and "falsey" make sense when there is an 'Is' prefix and implies a question.

I think the semantics here need to communicate the intent of generating the a || b pattern first and foremost. Something along these lines:

httpServer.Listen(Script.Ensure(Node.Process.Environment["port"], 8888))
httpServer.Listen(Script.OneOf(Node.Process.Environment["port"], 8888))
httpServer.Listen(Script.Or(Node.Process.Environment["port"], 8888))

Thoughts? Any ideas to share on a name that communicates the intended semantics? Might as well put some thought into it to justify the breaking change.

[strygo]

I personally prefer Truthy/Falsey since that naming maps to the concepts as I know them in the script world. Approaching the Script class and inspecting it in an object browser, I would have a better chance of knowing what Truthy/Falsey mean over Ensure/OneOf/Or. If I'm coming from a .NET background and don't understand the concepts, I'd probably gloss over Truthy/Falsey, which is probably desirable. :-)

any thoughts on just naming them:

if (Script.Truthy(false))
if (Script.Truthy(false, ""))
if (Script.Falsey(""))

I think of them more as casts to a boolean than a method that is doing some sort of high-level processing, so the "Is" prefix feels wrong. I say this because today, if you were to try to accomplish these semantics, you would do:

if ((bool)(object)value) // gnarly, I know ;-)

[nikhilk]

Interesting. I definitely was thinking of them as check methods rather than cast/conversion methods... hence the Is prefix to have them sit alongside IsNull, IsUndefined, etc. methods in classbrowser/intellisense.

Incidently, there is Script.Boolean for coercing to boolean, and avoiding double casts (and yes, the implementation is identical to IsTruthy). Given the existence of that method, I thought the Is methods made sense to have.

The pick-first-valid-value-script-shorthand syntax was the one needing more naming thought... because I agree Ensure/OneOf/Or don't do a great job communicating that. Well, maybe 'Or' is at list a bit suggestive?

[strygo]

Yeah, Or is the most obvious among them.

[jrockwood]

What about any of these?

Script.ToOr<T>(T value, T defaultValue, params T[] otherValues)
Script.ValueOrDefault<T>(T value, T defaultValue, params T[] otherValues)
Script.FirstValue<T>(T value1, T value2, params T[] otherValues)

Any of those communicates the semantics a little better, IMO.

[nikhilk]

ToOr seems similar enough to Or to suggest that Or is a reasonable name. I think ValueOrDefault would have worked nicely, but doesn't account for otherValues.

[dlev-]

Thanks for addressing this so quickly!

I think Value as it was documented would still be a useful function.

What should c#'s ?? generate? Right now it generates ||, but I think it should have the same behavior as the Value documentation.

[nikhilk]

I can buy wanting Value with the as-doc'd semantics (i.e. first non-null, non-undefined value) as well.

I think you're right on ?? ... given its a shortcut that checks against null values (null reference types or null nullables). Potentially a subtle breaking change (since false is now ok), but good observation, and something to address to match c# semantics.