Add value-based toString()s for model objects#452
Add value-based toString()s for model objects#452domesticmouse merged 1 commit intogooglemaps:masterfrom apjanke:model-value-based-toStrings
Conversation
| return PolylineEncoding.decode(points); | ||
| } | ||
|
|
||
| // Stick with the default toString; decoding the polyline to get a point count or representation |
There was a problem hiding this comment.
I'd suggest adding a toString that includes the encoded polyline.
|
I'm still waiting on tests for this PR, amirite? |
|
Yes. Sorry, been busy with the day job. :) |
|
All good =) |
|
Looks like I broke your PR. Once this is in I'll roll a release. Thanks! |
|
Rebased to fix conflict. Still busy with the day job. I may have time to add tests later tonight or tomorrow. |
|
Thanks Andrew, no rush |
|
Added tests. And what do you know, they caught a bug in one of my toString()s. The tests work by just running The tests don't check the content of the resulting string. Since these are human-readable strings for debugging, and don't have a fixed format or direct relation to an API, I don't think testing their content is necessary, or even appropriate. If you want, could do more thorough and isolated tests by just directly constructing various permutations of the model objects, including ones with |
|
Looks good to me. 👍 |
|
Excellent. Want to grab #463 before the release, too? |
Closes #450.
This PR adds value-based custom
toString()methods to all themodelobjects. This should be useful for debugging, including incorporation into log messages, and interactive use inside scripting environments like Matlab and Jython.Value-based toString()s are appropriate here because the model objects are value objects; their object identity doesn't matter as much as their contents, so the default
<ClassName>@<ObjectId>toString() output is not very useful.This PR affects user-visible behavior for existing classes, so you might want to sit on it a few days to allow other users to comment.
If this approach looks good to you, I'll add some tests.