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

The representation returned by value:toString is not human readable #296

Closed
sameerajayasoma opened this issue Aug 7, 2019 · 7 comments
Closed
Assignees
Labels
Area/LangLib Relates to lang.* libraries design/usability Design does not work well for some tasks
Milestone

Comments

@sameerajayasoma
Copy link
Contributor

Consider the following example:

type Person record {
    string name;
    string country?;
    Address addr;
    string[] otherNames;
};

type Address record {
    string line1;
    string line2 = "";
    string city;
    string postalCode;
};

public function main(string... args) {
    Person p = {name: "Sameera", "county":"England", 
                    otherNames: ["Madushan", "Jayasoma"],
                    addr: {
                        line1: "1234 Skyline st", 
                        city: "Mountain View", 
                        postalCode: "97803" 
                    } 
                };
    io:println(p.toString());
}

The output is:

name=Sameera county=England otherNames=Madushan Jayasoma addr=line2= line1=1234 Skyline st city=Mountain View postalCode=97803

This output is not human-friendly IMO. It is hard to tell the difference between a string with white spaces vs. an array of values. This representation has a flat structure, whereas the actual value has nested records, etc.

@sameerajayasoma sameerajayasoma added design/usability Design does not work well for some tasks Area/LangLib Relates to lang.* libraries labels Aug 7, 2019
@jclark
Copy link
Collaborator

jclark commented Aug 8, 2019

The design of toString follows this logic:

  1. toString(s) is s for any string s
  2. toString on a container recursively calls toString on the members
  3. toString on a list uses toString on the members with a separator in between
  4. space is the simplest, most neutral separator for list to use
  5. toString on a map should be the entries of the map separated in the same as a string
  6. an entry should be toString for the key, then a separator, then toString for the value
  7. the best separator for key/value is =

At which step do you disagree with the logic?

My view is that toString gives a simple, direct conversion of the value into a string. A function that was good for displaying arbitrarily complex structures would be very different from toString. In particular point 1 would not be true for such a function. It would be much more like toJsonString.

In the future, I expect us to define a Ballerina equivalent of JSON; as part of that there would be a function that converts an anydata value to a string in that format, which will clearly show an arbitrarily complex anydata structure, but neither point 1 nor point 2 would hold for it.

@jclark
Copy link
Collaborator

jclark commented Aug 8, 2019

Maybe something like the following would help (without losing the good points of toString):

  • if a member of a container (list or mapping) is a structural value, then toString on that container will follow that member by a newline rather than a space

@jclark jclark added the incompatible Resolving issue may not be backwards compatible label Aug 8, 2019
@jclark jclark added this to the 2019R3 milestone Aug 8, 2019
@sameerajayasoma
Copy link
Contributor Author

sameerajayasoma commented Aug 8, 2019

Here is another proposal for the design of toString.

  1. toString(s) is s for any string s
  2. toString on a container recursively calls toString on the members
  3. toString on a list uses toString on the members with a separator in between and enclosed by [ and ]
  4. space is the simplest, most neutral separator for list to use
  5. toString on a map should be the entries of the map separated in the same as a string and enclosed by { and }
  6. an entry should be toString for the key, then a separator, then toString for the value
  7. the best separator for key/value is =

With this design you get the following output:

{name=Sameera county=England otherNames=[Madushan Jayasoma] addr={line2= line1=1234 Skyline st city=Mountain View postalCode=97803}}

I think simply wrapping lists with [, ] and maps with {,} make this a bit more readable IMO.

@jclark
Copy link
Collaborator

jclark commented Aug 9, 2019

I strongly dislike that approach.

Using [] and {} is programming language syntax, not normal human, written language. A string conversion approach that is using such an approach should do it properly and generate something that can be parsed unambiguously. If you want this, then use toJsonString (or whatever it's native Ballerina equivalent).

toString is doing something fundamentally different: it is using the conventions of normal, human, written language, which uses punctuation and white-space to separate things.

toString does not work well for all cases, but it works great for simple lists of tokens, such as are often contained within strings in JSON or XML attributes or content. If toString wrapped with [], it would be useless for those cases.

@sameerajayasoma
Copy link
Contributor Author

My main concern with the current approach is that it is not human readable, even though we say it is human readable. What I am striving for is to output a human-readable representation of Ballerina values. The current string representation is not useful IMO. I wouldn't use it because I don't see the structure even for simple structured values.

In, my proposal my intention was not to output a parsable representation but to make the current representation a little bit more readable.

If we stick to the current representation, I assume most people would use toJsonString() instead of toString().

@jclark
Copy link
Collaborator

jclark commented Aug 10, 2019

I am happy to tweak the design of toString, but I want to observe three requirements:

  1. The way toString handles simple values should not change
  2. The result of toString applied to a list of simple values should also not change i.e. it should be the result of toString on each member separated by spaces. toString is used in the language in only two places: for string template expressions and xml literal expressions. For those places (particularly XML attributes), this is definitely the most useful behaviour.
  3. The behaviour of toString as a whole should feel coherent

Subject to the above three points, whatever we can do to improve readability is good.

The best I have come up so far is as follows:

  • toString on a list does memberToString(v) on each member separated by spaces
  • toString on a mapping does toString(k)=memberToString(v) on each entry separated by spaces
  • memberToString(v) is the same as toString(v) except when v is a list, mapping or table, in which case it is ( toString(v) )

So your example

{name: "Sameera", "county":"England", 
                    otherNames: ["Madushan", "Jayasoma"],
                    addr: {
                        line1: "1234 Skyline st", 
                        city: "Mountain View", 
                        postalCode: "97803" 
                    } 
                };

would turn into

name=Sameera county=England otherNames=(Madushan Jayasoma) addr=(line1=1234 Skyline st city=Mountain View postalCode=978803)

This is using parentheses to control grouping, in the same way as mathematics. I think this is more coherent with overall design of toString than using bits of Ballerina/JSON syntax.

@jclark jclark added the status/discuss Needs discussion outside github comments label Aug 14, 2019
@jclark jclark self-assigned this Aug 15, 2019
@jclark
Copy link
Collaborator

jclark commented Aug 28, 2019

I had a long discussion with @sanjiva about this, and we came to the conclusion that it was better not to make any change here.

The right solution to this problem is ti have the function described in #159 (we need to think of a good name for it). That function will always do a better job of providing a human readable representation of an arbitrary value because it does not have the constraint that toString(s) == s for any string s, which is fundamental to toString. So we believe the right solution is to prioritize #159, and then educate users about the appropriate uses of toString.

I will clarify the docs for toString accordingly.

@jclark jclark removed incompatible Resolving issue may not be backwards compatible status/discuss Needs discussion outside github comments labels Aug 28, 2019
@jclark jclark closed this as completed in d5da1bc Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/LangLib Relates to lang.* libraries design/usability Design does not work well for some tasks
Projects
None yet
Development

No branches or pull requests

2 participants