[haiku-development] Re: DeskCalc Improvements (was need strtold() function)
- From: John Scipione <jscipione@xxxxxxxxx>
- To: haiku-development@xxxxxxxxxxxxx
- Date: Thu, 31 Dec 2009 03:30:41 -0500
On Thu, Dec 31, 2009 at 3:25 AM, John Scipione <jscipione@xxxxxxxxx> wrote:
> On Wed, Dec 30, 2009 at 3:23 PM, John Scipione <jscipione@xxxxxxxxx> wrote:
>>> I think it would be better if
>>> a) You used BString API instead of copying the string to a char[] buffer
>>> (for example you can use BString::CountChars to get the length accounting
>>> for multibyte characters),
>>> b) You adapted the length of the displayed number to the window size.
>>>
>>> For number formatting, the Locale Kit should be taking care of it, have a
>>> look at headers/os/locale/FloatFormat.h . This is experimental API so :
>>> - Some of the functions present in the header may be buggy or do nothing at
>>> all,
>>> - You may need things that aren't available in this API.
>>> In any case, please let me know and I will complete the locale kit as you
>>> need.
>>>
>>> --
>>> Adrien Destugues / PulkoMandy
>>> http://pulkomandy.ath.cx
>>>
>>
>> On point a (should use BString API) I agree, I am sure there is a
>> better way to do what I am trying to do using existing classes rather
>> than resorting to sprintf and friends on a regular char array.
>> However, me not being a BeOS/Haiku expert I went with what I know. If
>> the FloatFormat class can do a better job than sprintf() and friends
>> at rounding numbers and formatting them as exponentials, etc. than I
>> am all for it. I'll work on it and post a revised patch.
>>
>> ---- Long rant follows, exec summary: resizing needs work ----
>>
>> On point b (adapt the length of the displayed number to the window
>> size) I have to disagree. It is too hard a problem and not worth it to
>> try and fit the result in whatever arbitrary window size is chosen. I
>> would have to recalculate the result as you resize which is clunky and
>> strange. A better method is to pick some precision and make sure that
>> the result fits in the display window no matter how big or small you
>> make the app. I don't like how when you resize the window less digits
>> fit. Look at any other calculator apps out there and you'll see that
>> they all use a fixed width and don't resize the input window.
>>
>> I like the fact that you can resize the window and it scales the font
>> but I think it should scale the font in such a way that the result
>> always fits in the display window. Perhaps a solution would be to
>> disable resizing and have two "modes" -- small mode and big mode. You
>> toggle between them by clicking the double block icon in the right
>> hand side of the title bar. In either mode you get the same number of
>> digits of result but in big mode the font is, well, bigger. Another
>> more difficult but perhaps better solution would be to scale the font
>> gradually as you resize.
>>
>> Either way the result has to be rounded at some point so that it fits
>> in the window.
>>
>> John Scipione
>>
>
> I rewrote my DeskCalc changes so that I would not be doing any char*
> manipulation. To achieve this had to extend the ExpressionParser Class
> with two new methods: EvaluateToStandard() and EvaluateToScientific().
> Each of these methods takes an int32 argument to specify the number of
> digits to grab. Trailing zeros after the decimal point are removed.
> Please review my patch below. Should patch from the haiku/ directory.
>
> Index: headers/private/shared/ExpressionParser.h
> ===================================================================
> --- headers/private/shared/ExpressionParser.h (revision 34828)
> +++ headers/private/shared/ExpressionParser.h (working copy)
> @@ -45,6 +45,11 @@
> void
> SetSupportHexInput(bool enabled);
>
> BString Evaluate(const char*
> expressionString);
> + BString EvaluateToStandard(
> + const char* expressionString,
> + int32 decimalPlaces);
> + BString
> EvaluateToScientific(const char*
> + expressionString, int32 decimalPlaces);
> int64 EvaluateToInt64(const
> char* expressionString);
> double EvaluateToDouble(const
> char* expressionString);
>
> Index: src/apps/deskcalc/CalcView.cpp
> ===================================================================
> --- src/apps/deskcalc/CalcView.cpp (revision 34828)
> +++ src/apps/deskcalc/CalcView.cpp (working copy)
> @@ -894,6 +894,9 @@
> void
> CalcView::Evaluate()
> {
> + const double STD_NOTATION_UPPER = 1e16;
> + const double STD_NOTATION_LOWER = 1e-16;
> +
> BString expression = fExpressionTextView->Text();
>
> if (expression.Length() == 0) {
> @@ -903,21 +906,39 @@
>
> _AudioFeedback(false);
>
> + ExpressionParser parser;
> + double value = 0.00;
> + BString result;
> +
> // evaluate expression
> - BString value;
> -
> try {
> - ExpressionParser parser;
> - value = parser.Evaluate(expression.String());
> + value = parser.EvaluateToDouble(expression.String());
> } catch (ParseException e) {
> BString error(e.message.String());
> error << " at " << (e.position + 1);
> - fExpressionTextView->SetText(error.String());
> - return;
> + // just print error
> + result.SetTo("error");
> + fExpressionTextView->SetExpression(result.String());
> + return;
> }
>
> + if (value == 0) {
> + result.SetTo("0");
> + fExpressionTextView->SetExpression(result.String());
> + return;
> + } else if (((value < STD_NOTATION_UPPER)
> + && (value > STD_NOTATION_LOWER))
> + || ((value > -STD_NOTATION_UPPER)
> + && (value < -STD_NOTATION_LOWER))) {
> + // Standard Notation
> + result = parser.EvaluateToStandard(expression.String(), 16);
> + } else {
> + // Scientific Notation
> + result = parser.EvaluateToScientific(expression.String(), 12);
> + }
> +
> // render new result to display
> - fExpressionTextView->SetExpression(value.String());
> + fExpressionTextView->SetExpression(result.String());
> }
>
>
> Index: src/kits/shared/ExpressionParser.cpp
> ===================================================================
> --- src/kits/shared/ExpressionParser.cpp (revision 34828)
> +++ src/kits/shared/ExpressionParser.cpp (working copy)
> @@ -317,6 +317,14 @@
> BString
> ExpressionParser::Evaluate(const char* expressionString)
> {
> + return EvaluateToStandard(expressionString, kMaxDecimalPlaces);
> +}
> +
> +
> +BString
> +ExpressionParser::EvaluateToStandard(const char* expressionString,
> + int32 decimalPlaces)
> +{
> fTokenizer->SetTo(expressionString);
>
> MAPM value = _ParseBinary();
> @@ -327,11 +335,14 @@
> if (value == 0)
> return BString("0");
>
> - char* buffer = value.toFixPtStringExp(kMaxDecimalPlaces, '.', 0, 0);
> + if (decimalPlaces <= 0)
> + decimalPlaces = kMaxDecimalPlaces;
> +
> + char* buffer = value.toFixPtStringExp(decimalPlaces, '.', 0, 0);
> if (buffer == NULL)
> throw ParseException("out of memory", 0);
>
> - // remove surplus zeros
> + // remove trailing decimal zeros
> int32 lastChar = strlen(buffer) - 1;
> if (strchr(buffer, '.')) {
> while (buffer[lastChar] == '0')
> @@ -343,6 +354,50 @@
> BString result(buffer, lastChar + 1);
> free(buffer);
> return result;
> +}
> +
> +
> +BString
> +ExpressionParser::EvaluateToScientific(const char* expressionString,
> + int32 decimalPlaces)
> +{
> + fTokenizer->SetTo(expressionString);
> +
> + MAPM value = _ParseBinary();
> + Token token = fTokenizer->NextToken();
> + if (token.type != TOKEN_END_OF_LINE)
> + throw ParseException("parse error", token.position);
> +
> + if (value == 0)
> + return BString("0");
> +
> + if (decimalPlaces <= 0)
> + decimalPlaces = kMaxDecimalPlaces;
> +
> + // max number of characters possible for 32 bits of decimal places is
> + // decimal places + 15 for -0.9999999....E+2147483647
> + // 3 + decimal digits + 2 + 10 = decimalPlaces + 15
> + char buffer[decimalPlaces + 15];
> + value.toString(buffer, decimalPlaces);
> + if (buffer == NULL)
> + throw ParseException("out of memory", 0);
> +
> + BString result(buffer, decimalPlaces + 15);
> +
> + // remove trailing decimal zeros
> + if (result.FindFirst('.') != B_ERROR) {
> + int32 offset = result.FindLast('E');
> + if (offset != B_ERROR) {
> + offset--; // go to character before 'E'
> + while(result[offset] == '0') {
> + result = result.Remove(offset, sizeof(char));
> + offset--;
> + }
> + if(result[offset] == '.')
> + result = result.Remove(offset, sizeof(char));
> + }
> + }
> + return result;
> }
>
Minor bug in patch:
if (decimalPlaces <= 0)
decimalPlaces = kMaxDecimalPlaces;
should read:
if (decimalPlaces < 0)
decimalPlaces = kMaxDecimalPlaces;
Not worth posting another patch, but 0 decimal places is valid.
Other related posts:
