- What is your evaluation of the design? It follows the design of the equivalent facility in std::, so this feels moot. No complaints with that part. My only complaint is that the semantics of boost::charconv::from_chars and boost::charconv::from_chars_strict seem backward to me. I know this was mentioned by other reviewers. I think that with near-identity to the standard version, having the same (or corresponding name within boost::) should imply the same semantics. This is classic adherence to The Principle of Least Surprise. I would much prefer that the varying version have the different name, and that the one with the standard's semantics should be from_chars. I also encourage the authors to track any changes to std::from_chars with boost::charconv::from_chars. - What is your evaluation of the implementation? I mean, there's a lot of it. I'm sure it's all necessary, but I only looked at about 10% of it. What I saw seemed reasonable and well organized. - What is your evaluation of the documentation? It's great. The library is smallish, and the docs are correspondingly bite-sized, with plenty of examples. I might add a little more front matter that explains why someone should use this library. The docs get there, but putting it in the first paragraph is better. Busy people will blow right past everything that isn't in the first paragraph or two. - What is your evaluation of the potential usefulness of the library? High. I think the fact that this is a C++17 feature that we still can't reliably use says it all. The fact that it's about the same as the std:: implementation, or sometimes a little faster, is even better. - Did you try to use the library? With what compiler? Did you have any problems? Yes. I tried it with GCC 12, in a small test program just to see it go. No issues. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I spent about 3 hours, mostly looking at code. I also read all the docs and gave it a quick try. - Are you knowledgeable about the problem domain? Not particularly. I have written lots and lots of parsers, but never a simple low-level one like those in charconv. Zach
On Wednesday, January 24, 2024 at 08:01:32 PM GMT+1, Zach Laine via Boost
wrote: > - What is your evaluation of the design?> It follows the design of the equivalent facility in std::, so this feels moot. No complaints with that part. My only complaint is that the semantics of boost::charconv::from_chars and boost::charconv::from_chars_strict seem backward to me. I know this was mentioned by other reviewers. I think that with near-identity to the standard version, having the same (or corresponding name within boost::) should imply the same semantics. This is classic adherence to The Principle of Least Surprise. I would much prefer that the varying version have the different name, and that the one with the standard's semantics should be from_chars. I also encourage the authors to track any changes to std::from_chars with boost::charconv::from_chars. - What is your evaluation of the implementation?
I mean, there's a lot of it. I'm sure it's all necessary, but I only looked at about 10% of it. What I saw seemed reasonable and well organized.
- What is your evaluation of the documentation?
It's great. The library is smallish, and the docs are correspondingly bite-sized, with plenty of examples. I might add a little more front matter that explains why someone should use this library. The docs get there, but putting it in the first paragraph is better. Busy people will blow right past everything that isn't in the first paragraph or two.
- What is your evaluation of the potential usefulness of the library?
High. I think the fact that this is a C++17 feature that we still can't reliably use says it all. The fact that it's about the same as the std:: implementation, or sometimes a little faster, is even better.
- Did you try to use the library? With what compiler? Did you have any problems?
Yes. I tried it with GCC 12, in a small test program just to see it go. No issues.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
I spent about 3 hours, mostly looking at code. I also read all the docs and gave it a quick try.
- Are you knowledgeable about the problem domain?
Not particularly. I have written lots and lots of parsers, but never a simple low-level one like those in charconv.
Zach Thank you Zach for your clear, terse review.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
On Wednesday, January 24, 2024 at 08:01:32 PM GMT+1, Zach Laine via Boost
wrote: Zach Thank you Zach for your clear, terse review. Hi Zach, could you please explicitly state ifyou accept or reject the proposed Boost.Charconv? I apologize, this is a bit pedantic and my initialpost did not have this clear query.
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
My only complaint is that the semantics of boost::charconv::from_chars and boost::charconv::from_chars_strict seem backward to me. I know this was mentioned by other reviewers. I think that with near-identity to the standard version, having the same (or corresponding name within boost::) should imply the same semantics. This is classic adherence to The Principle of Least Surprise. I would much prefer that the varying version have the different name, and that the one with the standard's semantics should be from_chars.
As the reviews/comments come in I am keeping track of this. Right now it is 2 for change, 1 for leave. I like that our default will not change behavior, but I understand the argument of Least Surprise.
I also encourage the authors to track any changes to std::from_chars with boost::charconv::from_chars.
Yes, whichever mode follows the standard will be updated accordingly when the LWG issue is closed out. Thank you for taking the time to review the library. I appreciate the feedback. Matt
participants (3)
-
Christopher Kormanyos
-
Matt Borland
-
Zach Laine