|
|
|
Return what 3.4 does, most likely undefined
I think null seems more appropriate. Returning null means some thought went into the decision rather than letting it be undefined.
The methods in question are getNodeName(), getNodeAttribute(), getNodeAttributes(), and getNodeText(). Right now these are undefined if p=null. How about this: - Display a warning message if debugging is enabled (this should also happen for some setters) - return null I like the reasoning, but:
In this case, when p == null, it means there is no node being pointed to, so I thing `undefined` is correct. It is telling you that "I couldn't even consider giving you an answer, because, basically, there is no question". Consider that when p != null, except for getNodeName, it is possible that the answer is `null`, meaning the node does not have attributes or text. It is plausible that this is a useful distinction. I also agree there should be a debugger warning. Remember to enclose it: if ($debug) { Debug.warn(...); } so that it is compiled away in non-debug. Change 20070802-Philip-2 by Philip@Philip-DC on 2007-08-02 11:34:48 EST
in /cygdrive/f/laszlo/svn/src/svn/openlaszlo/branches/wafflecone for http://svn.openlaszlo.org/openlaszlo/branches/wafflecone Summary: Fix how LzDatapointer handles null values New Features: Bugs Fixed: Technical Reviewer: ben QA Reviewer: (pending) Doc Reviewer: (pending) Documentation: Release Notes: Details: The decision is to return undefined when the internal pointer is null (same as 3 .4). This was the current behavior of wafflecone. I added warning messages in de bug mode whenever this condition is found. The new test file, lztest-lzdatapoint er.lzx, only tests the undefined behavior of LzDatapointer. Tests: Run new unit test, /test/lztest/lztest-lzdatapointer.lzx Files: A test/lztest/lztest-lzdatapointer.lzx M WEB-INF/lps/lfc/data/LzDatapointer.lzs Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20070802-Philip-2.tar (wafflecone branch local build r5986)
WARNING: getNodeName: p is null in LzDatapointer WARNING: getNodeAttributes: p is null in LzDatapointer WARNING: getNodeAttribute: p is null in LzDatapointer WARNING: getNodeText: p is null in LzDatapointer WARNING: addNode: p is null in LzDatapointer WARNING: deleteNode: p is null in LzDatapointer WARNING: serialize: p is null in LzDatapointer WARNING: setNodeName: p is null in LzDatapointer WARNING: setNodeAttribute: p is null in LzDatapointer WARNING: deleteNodeAttribute: p is null in LzDatapointer WARNING: setNodeText: p is null in LzDatapointer INFO: Passed all suites. Warnings are correct. |
||||||||||||||||||||||||||||||||||||||||||||||||||||
p7w: Could fall out from actually trying to doc the API.
maxlaszlo: Emulating Flash seems like the right thing to do... Or break apps and emulate W3C more closely..
p7w: I'm not the data expert, I think we have swf compatibility, so this is mainly a doc/design issue that can be deferred forever.
p7w: Emulating flash just means doing what flash does when you don't supply the right args, I don't think that is 'right', just expedient.
maxlaszlo: Agreed - we can defer this one for a while