Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolves #2247, adds a shim to support custom manifest 'identifier' #2332

Merged
merged 4 commits into from
Jan 22, 2019

Conversation

brian-learningpool
Copy link
Member

This ensures 'adapt_scorm' will be used as a default if the corresponding property is missing.

This ensures 'adapt_scorm' will be used as a default if the corresponding property is missing.
grunt/config/replace.js Outdated Show resolved Hide resolved
grunt/config/replace.js Outdated Show resolved Hide resolved
Declare _advancedSettings as an object if it does not exist.
@moloko moloko merged commit db636da into accessibilityv4 Jan 22, 2019
@moloko moloko deleted the issue/2247 branch January 22, 2019 16:51
@oliverfoster
Copy link
Member

oliverfoster commented Jan 22, 2019

The hasOwnProperty check always seems like a an overkill when you can use falsies to do the same but with less code. JSON objects don't have an inheritance chain to speak of which is part of the usecase of hasOwnProperty.

value !!value
false false
0 false
NaN false
null false
undefined false
“” (empty string) false
” ” (white space) true
[] true
{} true

The former should suffice as an existence check, as if the property is undefined because it doesn't exist, then it'll fail all the same:

if (jsonObject.propertyName) {
  // this will return true if the property contains anything that doesn't equate to false
  // it doesn't care if the property has been inherited, which it shouldn't have been because we're dealing with JSON
}
if (jsonObject.hasOwnProperty('propertyName')) {
  // this will return true if the property has ever been declared regardless of the value assigned, it may still be undefined
  // if the property has been inherited it will be ignored here, but one shouldn't have been because we're dealing with JSON
}

This works because the default value is only assigned if the jsonObject.propertyName is a falsie, in effect the line is skipped if jsonObject.propertyName has a non-false value:

jsonObject.propertyName = jsonObject.propertyName || defaultValue;

@oliverfoster
Copy link
Member

Object references:

var a = { 
  b: { 
    c: { 
        value: 1, 
        value1: 2 
    } 
  } 
};

If we're going to do a lot of work on object c, then we can store a temporary reference to it in a new variable rather than having to descend through its parents each time to access it.

Such that:

a.b.c.value = 2;
a.b.c.value1 = 3;

// and

var tempRef = a.b.c;
tempRef.value = 2;
tempRef.value1 = 3;

// are equivalent

The same object has been modified in both cases as objects are passed by reference.
a.b.c.value === 2 && a.b.c.value1 === 3 regardless as to which pattern is used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants