History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: LPP-2760
Type: Task Task
Status: Closed Closed
Resolution: Fixed
Priority: P1 P1
Assignee: Unassigned
Reporter: Benjamin Shine
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
OpenLaszlo

Decide how the data API's should handle null values

Created: 25/Sep/06 03:50 PM   Updated: 09/Aug/07 06:54 PM
Component/s: Kernel - DHTML
Affects Version/s: Legals PR4
Fix Version/s: 4.0.5WaffleCone

Time Tracking:
Not Specified

Severity: Minor
Fixed in Change#: 5,968
Runtime: N/A
Fix in hand: False


 Description  « Hide
In LzDatapointer.lzs, what do we want to return if this.p is null? Specifically, do we want to return a type-safe null ("" or null) or just undefined?

 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Jim Grandy - 16/Oct/06 01:41 PM
p7w: Could be solved by emulating swf. But perhaps some thought should be given.
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

P T Withington - 27/Jul/07 10:47 AM
Return what 3.4 does, most likely undefined

Philip Romanik - 31/Jul/07 07:51 AM
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

P T Withington - 01/Aug/07 10:06 AM
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.

Philip Romanik - 08/Aug/07 01:43 PM
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: LPP-2760

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

Mamye Kratt - 09/Aug/07 06:54 PM
(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.