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

Revisit toString #499

Closed
jclark opened this issue Apr 25, 2020 · 11 comments
Closed

Revisit toString #499

jclark opened this issue Apr 25, 2020 · 11 comments
Assignees
Labels
design/usability Design does not work well for some tasks incompatible Resolving issue may not be backwards compatible

Comments

@jclark
Copy link
Collaborator

jclark commented Apr 25, 2020

Given we are doing 2.0, I think we should revisit the design of toString to see if we can come up with something that works better than our current approach.

@jclark jclark self-assigned this Apr 25, 2020
@jclark jclark added this to the 2020R3 milestone Apr 25, 2020
@jclark jclark added the design/usability Design does not work well for some tasks label Apr 25, 2020
@jclark
Copy link
Collaborator Author

jclark commented Apr 25, 2020

Need to consider how this relates to #159.

@jclark
Copy link
Collaborator Author

jclark commented Aug 6, 2020

Also related to #135.

@jclark jclark added incompatible Resolving issue may not be backwards compatible status/discuss Needs discussion outside github comments labels Aug 6, 2020
@jclark
Copy link
Collaborator Author

jclark commented Aug 21, 2020

New design is as follows.

The spec would define an abstract operation ToString(v, style), where three styles are supported:

  • expression style: produces a string that uses a subset of Ballerina expression syntax; guarantees that
    • for a value v that is anydata, the string can be parsed and will produce a value that is == to v
      • if value is acyclic, result will be Ballerina expression
    • for a value v that non anydata, string is distinguishable from value produced for v that is anydata
  • informal style: similar to expression style, but
    • avoids some more esoteric Ballerina syntax
    • does not preserve all the distinctions that expression style does
  • direct style: converts a value to string in a more direct way, for basic types where this makes sense
    • more direct means converting the value to a string rather than describing the value using a string
      • most importantly, direct style applied to a string leaves string as is
      • xml is represented in XML format
    • falls back to informal style, for basic types where this doesn’t make sense, e.g. for collections
    • for object, uses toString method of object if there is one
      • should probably be required to be isolated

This design is similar to Python:

  • Python’s str(x) function corresponds to direct style
  • Python’s repr(x) functions corresponds to expression style

Python doesn’t have the third style; this leads to weirdness when applying str(x) to collections, in that the members are output in a way that has an inappropriate level of detail.

The value:toString function would be defined to use direct style.

A new value:toBalString will use expression style. Informal style would probably be available by using arguments to value:toBalString; other options can be added over time (e.g. for pretty printing).

j.toBalString() when j is json is guaranteed not to have newlines; d.toBalString() may have newlines if d contains tables or XML.

These functions should be robust in the presence of cycles. The implementation of toString would have a stack of the structures/objects that are being converted; toString when applied to some object, would first check whether the object is on the stack; if so, then there’s a cycle; otherwise, when it calls toString on members, it does so with itself added to the top of the stack. For now, we have the limitation that there is no mechanism for user-defined toString on objects to deal with cycles.

The following table summarizes the behaviour of the abstract operation ToString(v, style)

Type of v style
expression informal direct
nil () null
true true
int 2
float 1.0
float:NaN NaN
float:Infinity Infinity
decimal 1d 1
1.20d 1.20
string "xyz" xyz
xml xml`<d>t</d>` `<d>t</d>` <d>t</d>
array [X,Y]
X, Y expression style X,Y informal style
map {"x":X,"y":Y}
X, Y expression style X,Y informal style
table table key(k) [R1,R2] [R1,R2]
R1,R2 expression style R1,R2 informal style
error error D1&D2 (M,C,F1=V1,F2=V2)
V1, V2 expression style, D is {module}local-name, C expression style V1, V2 informal style, D is local-name, C informal style
object w/o toString object D1&D2 U U is an identifier that uniquely identifies the object (based on ===)
object with toString object S S
cycle ...[N] ...
N is 0-based index down path from root object to object where cycle is detected

@jclark jclark added status/pending Design is agreed and waiting to be added and removed status/discuss Needs discussion outside github comments labels Aug 21, 2020
@lasinicl
Copy link
Contributor

@jclark
Consider applying toString() on an error type value

public type FirstError distinct error;
error cause = error("Account not found", accountID = 123);
FirstError err = FirstError("Reason1", cause, detail = "Failed to get account balance");
string str = err.toString();

Is the expected value of str, the following?

error FirstError (message=Reason1, cause=error error (message=Account not found, detail={"accountID":123}), detail={"detail":"Failed to get account balance"})

For the example given below,

error cause = error("Some issue");
http:FailoverAllEndpointsFailedError err = http:FailoverAllEndpointsFailedError("failoverMessage", failoverErrors = cause);
string str = err.toString();

can this be the expected outcome?

error {ballerina/http}FailoverAllEndpointsFailedError (message=failoverMessage, detail={"failoverErrors":error error (message=Some issue)})

@jclark
Copy link
Collaborator Author

jclark commented Sep 11, 2020

The entry for error in the table isn't quite right: I just edited it.

Note that your syntax is no longer right in Swan Lake. It's

error cause = error("Account not found", accountID = 123);
FirstError err = error FirstError("Reason1", cause, detail = "Failed to get account balance"); // error keyword needed here too

The toString output should look like the constructor:

error FirstError ("Reason1", error("Account not found", accountID=123), detail="Failed to get account balance");

Specifically:

  • omit the distinct name if there isn't one
  • omit the cause if it's nil

The expression style includes the module name in the distinct type-id.

@mohanvive
Copy link

mohanvive commented Sep 11, 2020

@jclark a quick clarification on map.

Does map contains a space after after the ":" (and before) the value?,

For example,

{"name": "Jane", "age": 25, "spouse": "John", "gender": "female"}

@jclark
Copy link
Collaborator Author

jclark commented Sep 12, 2020

I definitely wouldn't put a space before :. I don't have a strong opinion about after :, but , and : should be treated consistently as regards spacing (for both mappings and lists) So either:

{"name": "Jane", "age": 25, "spouse": "John", "gender": "female"}
[1, 2, 3]

as you wrote, or

{"name":"Jane","age": 25,"spouse": "John","gender":"female"}
[1,2,3]

I slightly prefer with spaces for readability. Which do you prefer?

@mohanvive
Copy link

mohanvive commented Sep 12, 2020

Actually I prefer the second option which you suggested (without spaces), just intuitive.

But shall we make a quick decision on this because there are some dependent changes pending?

@jclark
Copy link
Collaborator Author

jclark commented Sep 12, 2020

I don’t have a strong opinion and I don’t think it matters very much. Do whichever the implementation team prefers and let me know which you decide.

@mohanvive
Copy link

Sure James. We are moving forward with the option #2 (without spaces).

@jclark
Copy link
Collaborator Author

jclark commented Sep 12, 2020

OK, I updated the table above accordingly: toString for error also should not add space after string.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design/usability Design does not work well for some tasks incompatible Resolving issue may not be backwards compatible
Projects
None yet
Development

No branches or pull requests

3 participants