arteconceito commented on issue #14973:
URL: https://github.com/apache/arrow/issues/14973#issuecomment-1363282688

   Before the conversation goes into a different path other that the main point 
of why this issue was raised, let me add a bit of context to why I opened this 
issue as a bug.
   
   As in the example I gave on [my OP 
above](https://github.com/apache/arrow/issues/14973#issue-1498642827), the 
output of `table.toString()` is too similar to a JSON string. Similar enough to 
induce one to take it as a "helper" method for getting the actual table as a 
serialised JSON string.
   
   This is exactly what happened with two other devs on my team that authored 
the code I was assigned to fix. Once we started having dates on those tables, 
the app broke as I then figured why. It was exactly due to the fact that 
`.toString()` was used to get the table as JSON and because the `.toString()` 
method does not quote the date strings (as also mentioned in my original OP).
   
   So, as per the [MDN's 
definition](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/toString)
 of `Object.prototype.toString()`, the `.toString()` should not only be used to 
return a string representation but is also meant to be overridden for custom 
type conversion. Below is an example of a "native" string representation (yes, 
it's not that helpful),
   ```javascript
   const arrayOfObjects = [{ name: 'John', dob: new Date() }, { name: 'Jane', 
dob: new Date() }];
   
   console.log(arrayOfObjects);
   // "[object Object],[object Object]"
   ```
   
   Compared to this, the current `Table.toString()` implementation does a good 
job already and is actually following the "meant to be overridden for custom 
type conversion" part of the recommendation. I only meant to "adjust" it in a 
way that it would be backwards compatible, continue being human readable while 
also being JSON parseable as a nice addition.
   
   Regarding the part where you write...
   > `toJSON` has the challenge that we use it to produce js objects rather 
than actual JSON strings
   
   ... I get it that's how you intended it to be in the first place but it for 
sure, at least to me, makes one think it will return a JSON string, as its name 
suggests, and also because JSON on its most basic definition is a string. I 
know introducing changes here would be a breaking change but just in case, my 
suggestion would be `.valueOf()` following [MDN's 
definition](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/valueOf)
 on `Object.prototype.valueOf()`.
   
   Anyway... going back to the basics.
   The current `Table.toString()` already returns a JSON parseable string, why 
not also support `Date()` values ?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to