lua-users home
lua-l archive

[Date Prev][Date Next][Thread Prev][Thread Next] [Date Index] [Thread Index]


On Wed, Mar 5, 2014 at 1:35 PM, Andrew Starks <andrew.starks@trms.com> wrote:
> On Wed, Mar 5, 2014 at 10:06 AM, Roberto Ierusalimschy
> <roberto@inf.puc-rio.br> wrote:
>>> My motivation was to convey an experience. I was surprised to spend a
>>> couple of hours combing through source code that relied (wrongly) on
>>> `tostring` and thought that this feedback would be helpful.
>>
>> It was! BTW, can you name the libraries you had to patch?
>>
>> -- Roberto
>>
>
> I did them all at once. I will do some digging in git, but I can tell
> you that Penlight was one of them. (I've submitted a pull request,
> although patching for a "work 1" release is probably stretching
> things)
>
> I will also assume that you are not interested in the libraries which
> failed due to explicit version checks. Ex: "if not lua5.2 then --do
> old behavior" There were maybe 10 or so of those.
>
>
> --Andrew

I went back to look and then did more investigation.

I'm embarrassed to say that I may have counted Penlight twice as more
than 3 submodules modules had issues (data, comprehensions and
pretty... also maybe func)). I stopped trying to patch it after it
became clear that it would require me to know more about some of the
sub-libraries than I had time to analyze.

There was also an older date library (by Jas Latrix) with some edge
cases that were easy to clean up, given that `tostring` was used quite
consistently.

We had several in-house tests that failed, because we were asserting
that either an error code would return an exact string, that did not
include a .0, or because a string comparison of a pretty print did not
include a .0. This last category might have less weight, or it
probably proves that the approach is well considered. However,
sometimes "1.0" and "1" truly don't matter and having to trap both
(given that the number may be calculated in some cases and a literal
in others) adds code that is sometimes not otherwise valuable. Again,
this usually comes up when analyzing pretty output.

Your proposed change of retaining the old behavior for the concat
operator would have covered all but one of these cases, which is
table.concat. However, I do like consistency of "tostring" being used
for concat and print and everything else.

So, on the one hand, I found that the change created/exposed bugs in
software that were sometimes hard to find and were sometimes not
covered by a test and making `..` work as it did would help find the
most difficult cases to find, as far as I can see.

On the other hand, it's my opinion that `tostring` should be used
every time `..` is used and searching for `..` is pretty straight
forward.

Now that a number may print differently in given situations, explicit
formatting is more important. It was good practice and now there are
times where you must do it.

If you agree, then I would welcome a more compact way to format a
number for printing/concatenating/storing as a string.
`("%.f"):format(x)` (or what have you..) is not bad at all, however it
can get awkward to type and I type it much more often than I use to.
:) I have some ideas, but they are the obvious ones.


-Andrew