From fbcef7005de4436682072927f83000b502928d25 Mon Sep 17 00:00:00 2001 From: Hans Boehm Date: Mon, 27 Apr 2015 18:07:47 -0700 Subject: Fix strings, stability bug, easy UI & correctness issues Bug: 20625562 Bug: 20649711 Bug: 20561890 Bug: 20561528 Bug: 20442590 Bug: 15473140 Bug: 20503008 Bug: 20503007 - Improve timeout text. - Recalculate when Calculator is rotated, e.g. in error state, thus reproducing message. It's unclear this is good enough, but it's better. - Fix square root parsing. - Fix concatenation of numbers when pasting by adding explicit multiplication. - Display divide by zero error differently from other domain errors. - Improved advanced keypad layout of portrait-mode tablet calculator. - Improved overflow menu order. (More to be done.) - Report zero division as zero division when we can recognize it. - Switch to floating menus for copy/paste. Change-Id: I3875414f293e62a59b0e41f0de822f29bd5ac6a6 --- src/com/android/calculator2/BoundedRational.java | 8 ++++- src/com/android/calculator2/Calculator.java | 36 +++++++++---------- src/com/android/calculator2/CalculatorExpr.java | 42 +++++++++++++++++++---- src/com/android/calculator2/CalculatorResult.java | 8 +++-- src/com/android/calculator2/CalculatorText.java | 2 +- src/com/android/calculator2/Evaluator.java | 5 ++- 6 files changed, 70 insertions(+), 31 deletions(-) (limited to 'src') diff --git a/src/com/android/calculator2/BoundedRational.java b/src/com/android/calculator2/BoundedRational.java index 04cf86c..d95b52b 100644 --- a/src/com/android/calculator2/BoundedRational.java +++ b/src/com/android/calculator2/BoundedRational.java @@ -170,10 +170,16 @@ public class BoundedRational { return new BoundedRational(num,den).maybeReduce(); } + public static class ZeroDivisionException extends ArithmeticException { + public ZeroDivisionException() { + super("Division by zero"); + } + } + static BoundedRational inverse(BoundedRational r) { if (r == null) return null; if (r.mNum.equals(BigInteger.ZERO)) { - throw new ArithmeticException("Divide by Zero"); + throw new ZeroDivisionException(); } return new BoundedRational(r.mDen, r.mNum); } diff --git a/src/com/android/calculator2/Calculator.java b/src/com/android/calculator2/Calculator.java index ed5eb6b..df7c09c 100644 --- a/src/com/android/calculator2/Calculator.java +++ b/src/com/android/calculator2/Calculator.java @@ -239,19 +239,18 @@ public class Calculator extends Activity mEvaluator.clear(); } } + } else { + mCurrentState = CalculatorState.INPUT; + mEvaluator.clear(); } mFormulaText.setOnKeyListener(mFormulaOnKeyListener); mFormulaText.setOnTextSizeChangeListener(this); mFormulaText.setPasteListener(this); mDeleteButton.setOnLongClickListener(this); updateDegreeMode(mEvaluator.getDegreeMode()); - if (mCurrentState == CalculatorState.EVALUATE) { - // Odd case. Evaluation probably took a long time. Let user ask for it again - mCurrentState = CalculatorState.INPUT; - // TODO: This can happen if the user rotates the screen. - // Is this rotate-to-abort behavior correct? Revisit after experimentation. - } if (mCurrentState != CalculatorState.INPUT) { + // Just reevaluate. + redisplayFormula(); setState(CalculatorState.INIT); mEvaluator.requireResult(); } else { @@ -607,19 +606,20 @@ public class Calculator extends Activity // Evaluation encountered en error. Display the error. void onError(final int errorResourceId) { - if (mCurrentState != CalculatorState.EVALUATE) { - // Only animate error on evaluate. - return; + if (mCurrentState == CalculatorState.EVALUATE) { + setState(CalculatorState.ANIMATE); + reveal(mCurrentButton, R.color.calculator_error_color, + new AnimatorListenerAdapter() { + @Override + public void onAnimationEnd(Animator animation) { + setState(CalculatorState.ERROR); + mResult.displayError(errorResourceId); + } + }); + } else if (mCurrentState == CalculatorState.INIT) { + setState(CalculatorState.ERROR); + mResult.displayError(errorResourceId); } - - setState(CalculatorState.ANIMATE); - reveal(mCurrentButton, R.color.calculator_error_color, new AnimatorListenerAdapter() { - @Override - public void onAnimationEnd(Animator animation) { - setState(CalculatorState.ERROR); - mResult.displayError(errorResourceId); - } - }); } diff --git a/src/com/android/calculator2/CalculatorExpr.java b/src/com/android/calculator2/CalculatorExpr.java index 955fd7b..631be01 100644 --- a/src/com/android/calculator2/CalculatorExpr.java +++ b/src/com/android/calculator2/CalculatorExpr.java @@ -396,7 +396,23 @@ class CalculatorExpr { // TODO: We probably only need this for expressions consisting of // a single PreEval "token", and may want to check that. void append(CalculatorExpr expr2) { + // Check that we're not concatenating Constant or PreEval + // tokens, since the result would look like a single constant + int s = mExpr.size(); int s2 = expr2.mExpr.size(); + // Check that we're not concatenating Constant or PreEval + // tokens, since the result would look like a single constant, + // with very mysterious results for the user. + if (s != 0 && s2 != 0) { + Token last = mExpr.get(s-1); + Token first = expr2.mExpr.get(0); + if (!(first instanceof Operator) && !(last instanceof Operator)) { + // Fudge it by adding an explicit multiplication. + // We would have interpreted it as such anyway, and this + // makes it recognizable to the user. + mExpr.add(new Operator(R.id.op_mul)); + } + } for (int i = 0; i < s2; ++i) { mExpr.add(expr2.mExpr.get(i)); } @@ -558,12 +574,22 @@ class CalculatorExpr { case R.id.const_e: return new EvalRet(i+1, CR.valueOf(1).exp(), null); case R.id.op_sqrt: - // Seems to have highest precedence - // Does not add implicit paren - argVal = evalUnary(i+1, ec); - ratVal = BoundedRational.sqrt(argVal.mRatVal); - if (ratVal != null) break; - return new EvalRet(argVal.mPos, argVal.mVal.sqrt(), null); + // Seems to have highest precedence. + // Does not add implicit paren. + // Does seem to accept a leading minus. + if (isOperator(i+1, R.id.op_sub)) { + argVal = evalUnary(i+2, ec); + ratVal = BoundedRational.sqrt( + BoundedRational.negate(argVal.mRatVal)); + if (ratVal != null) break; + return new EvalRet(argVal.mPos, + argVal.mVal.negate().sqrt(), null); + } else { + argVal = evalUnary(i+1, ec); + ratVal = BoundedRational.sqrt(argVal.mRatVal); + if (ratVal != null) break; + return new EvalRet(argVal.mPos, argVal.mVal.sqrt(), null); + } case R.id.lparen: argVal = evalExpr(i+1, ec); if (isOperator(argVal.mPos, R.id.rparen)) argVal.mPos++; @@ -832,7 +858,9 @@ class CalculatorExpr { try { EvalContext ec = new EvalContext(degreeMode); EvalRet res = evalExpr(0, ec); - if (res.mPos != mExpr.size()) return null; + if (res.mPos != mExpr.size()) { + throw new SyntaxError("Failed to parse full expression"); + } return new EvalResult(res.mVal, res.mRatVal); } catch (IndexOutOfBoundsException e) { throw new SyntaxError("Unexpected expression end"); diff --git a/src/com/android/calculator2/CalculatorResult.java b/src/com/android/calculator2/CalculatorResult.java index ef62be3..72c3b54 100644 --- a/src/com/android/calculator2/CalculatorResult.java +++ b/src/com/android/calculator2/CalculatorResult.java @@ -107,8 +107,9 @@ public class CalculatorResult extends TextView { mCurrentPos = mScroller.getFinalX(); } mScroller.forceFinished(true); - CalculatorResult.this.cancelLongPress(); // Ignore scrolls of error string, etc. - if (!mScrollable) return true; + CalculatorResult.this.cancelLongPress(); + // Ignore scrolls of error string, etc. + if (!mScrollable) return true; mScroller.fling(mCurrentPos, 0, - (int) velocityX, 0 /* horizontal only */, mMinPos, MAX_RIGHT_SCROLL, 0, 0); @@ -134,7 +135,8 @@ public class CalculatorResult extends TextView { } @Override public void onLongPress(MotionEvent e) { - startActionMode(mCopyActionModeCallback); + startActionMode(mCopyActionModeCallback, + ActionMode.TYPE_FLOATING); } }); setOnTouchListener(mTouchListener); diff --git a/src/com/android/calculator2/CalculatorText.java b/src/com/android/calculator2/CalculatorText.java index 01f234b..1f91572 100644 --- a/src/com/android/calculator2/CalculatorText.java +++ b/src/com/android/calculator2/CalculatorText.java @@ -149,7 +149,7 @@ public class CalculatorText extends TextView implements View.OnLongClickListener @Override public boolean onLongClick(View v) { - startActionMode(mPasteActionModeCallback); + startActionMode(mPasteActionModeCallback, ActionMode.TYPE_FLOATING); return true; } diff --git a/src/com/android/calculator2/Evaluator.java b/src/com/android/calculator2/Evaluator.java index b4a73c6..25da744 100644 --- a/src/com/android/calculator2/Evaluator.java +++ b/src/com/android/calculator2/Evaluator.java @@ -343,7 +343,6 @@ class Evaluator { protected InitialResult doInBackground(Void... nothing) { try { CalculatorExpr.EvalResult res = mExpr.eval(mDm); - if (res == null) return null; int prec = 3; // Enough for short representation String initCache = res.mVal.toString(prec); int msd = getMsdPos(initCache); @@ -365,6 +364,10 @@ class Evaluator { initCache, prec, initDisplayPrec); } catch (CalculatorExpr.SyntaxError e) { return new InitialResult(R.string.error_syntax); + } catch (BoundedRational.ZeroDivisionException e) { + // Division by zero caught by BoundedRational; + // the easy and more common case. + return new InitialResult(R.string.error_zero_divide); } catch(ArithmeticException e) { return new InitialResult(R.string.error_nan); } catch(PrecisionOverflowError e) { -- cgit v1.2.3