Last active
September 7, 2015 02:33
-
-
Save joshuaebowling/98457ba11d743e146def to your computer and use it in GitHub Desktop.
Response to _Opportunity
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
// I'm going to leave my debugging code in so you can see how I prove to myself that I've resolved issues | |
/* I added the NAMESPACE declaration to troubleshoot. | |
* I understand this was only part of a larger | |
* codeblock that may or may not have declared NAMESPACE | |
*/ | |
var NAMESPACE; | |
if (!NAMESPACE || typeof NAMESPACE == 'undefined') { | |
NAMESPACE = { | |
_all_ids: new Object() | |
}; | |
var id = function (id) { | |
var persona = {}; | |
// shorthand reference | |
var _all_ids = NAMESPACE._all_ids; | |
// local var declaration | |
var _id = id; | |
/* because the id gets deleted from _all_ids on close() invocation, I imagine it should be added also. | |
* but I have no clue to what end numbers are added or deleted from this array. | |
*/ | |
// added terminating semi-colon to end of function | |
var getId = function () { | |
// use of local var | |
return _id; | |
}; | |
// assignment of function to returned object | |
persona.getId = getId; | |
// add the current _id to _all_ids | |
_all_ids[_id] = persona; | |
// local var declaration | |
var _closed = false; | |
// added terminating semi-colon to end of function | |
var close = function () { | |
delete _all_ids[getId()]; | |
// the line below references a prototypal property that doesn't seem to be in existence | |
// this._closed = true; | |
// the output below should prove that this._closed is an improper reference | |
console.log('according to the original code, this should be false, not undefined.', this._closed); | |
// instead, I'll reference the local property created above | |
_closed = true; | |
}; | |
persona.close = close; | |
return persona; | |
} | |
NAMESPACE['id'] = id; | |
} |
@posita,
see the repository
@posita,
Is it as simple as NAMESPACE._all_ids = {}
, or should we do something else? What are the benefits/drawbacks to such an approach (vs. another)?
Pros
- Simple is awesome, unless it's overly simplistic. What's more, it seems to work judging by the tests I've included in the repository.
Cons
- There's no protection from
_all_ids
getting overwritten or deleted accidentally. - There would be no event hooks, etc.
- There would be no facility for features such as
undo
.
My recommendation: There is precedent in the code to declare a local and return the local from a function, a la var getId = function() {}; persona.getId = getId;
This will take care of Cons 1 which is critical while Cons 2,3 are pre-optimizey and I've seen no indication such features are necessary so far.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
@posita,
I'll begin immediately.