allwpilib
allwpilib copied to clipboard
Improve `Color#toString()`/`Color8Bit#toString()` in Java
Is your feature request related to a problem? Please describe.
Colors can be stringified in a human-friendly way, but they aren't. They use the default Java Object#toString()
behavior of returning the simple class name with the memory address appended (easy to mistake for a hex color code!).
Describe the solution you'd like
Named colors should get special #toString()
values:
Color.kBlack.toString(); // "kBlack"
Other colors should get
new Color8Bit(0xff, 0xbb, 0xcc); // "#ffbbcc"
Additional context
https://discord.com/channels/176186766946992128/368993897495527424/960415849259368468
Mapping named colours back to their names would probably be a lot of work. Would just returning the hex code be reasonable too?
On a similar note, in RobotPy we have a __repr__
implementation for colours, which give something like Color8Bit(255, 187, 204)
(i.e. it returns an expression to reproduce the object, as is standard behaviour for repr()
in Python). I think a differing toString
would be reasonable too.
I don't think this should be exclusive to Java - no reason C++ shouldn't get a similar method.
The PR that closed this fixed the confusion, but doesn't deal with names for specialized colors.
It's just a variable, you can't really do that without reflection (not even sure if you can do it with that).
You could compare the color components with kBlack, for example, then print "kBlack" if they match. That could get expensive if there's a lot of predefined colors though. Maybe have a hash map from color components to pre-defined colors?
If you write a big switch statement, the compiler will do most of the work for you. But I agree that it might be more expensive than you might want to pay if you're e.g. logging the color to a CSV file every iteration or something like that. Maybe not include it in toString(), but add a function like toPrettyString()?
I was thinking of adding a string
constructor parameter that defaults to the RGB hex. That way it's cached for all objects, and not expensive at all.
Yeah that would work. Main downside is you’re forcing the overhead for hex conversion every time a color is constructed. Avoiding that might be micro-optimization, but could be done eg by storing a blank/null string for hex values and converting/caching it only on toString.
Sounds like a good idea.