Merge lp:~f-riccardo87/ubuntu-calculator-app/new-formula-structure into lp:~ubuntu-calculator-dev/ubuntu-calculator-app/old_trunk

[Riccardo Ferrazzo]

New formula implementation

Operations on the formula moved to a specific javascript class within another file.
Implementation of every preexisting function to use the new formula implementation.
C button now works as expected.
Some bugs corrected

[Gustavo Pichorim Boiko (boiko)]

108 + if(last.type === CALC.T_NUMBER || last.type === CALC.T_UNARY_MINUS){
109 + screenFormula[screenFormula.length - 1]._number += last.value.toString();
110 + } else {
111 + screenFormula.push({_text:'', _operation: last.value.toString(), _number: ''})

Would you mind adding a comment in the code explaining what is the use case being handle here? It will make it easier to understand and avoid wrong changes in the future.

review: Needs Fixing

[lp:~f-riccardo87/ubuntu-calculator-app/new-formula-structure]

47. By Riccardo Ferrazzo on 2013-04-10

comments added

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:47
http://91.189.93.125:8080/job/ubuntu-calculator-app-ci/9/
Executed test runs:
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-calculator-app-quantal-amd64-ci/9
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-calculator-app-raring-amd64-ci/9

Click here to trigger a rebuild:
http://91.189.93.125:8080/job/ubuntu-calculator-app-ci/9/rebuild

review: Approve (continuous-integration)

[Riccardo Ferrazzo (f-riccardo87)]

> 108 + if(last.type === CALC.T_NUMBER || last.type ===
> CALC.T_UNARY_MINUS){
> 109 + screenFormula[screenFormula.length - 1]._number +=
> last.value.toString();
> 110 + } else {
> 111 + screenFormula.push({_text:'', _operation:
> last.value.toString(), _number: ''})
>
> Would you mind adding a comment in the code explaining what is the use case
> being handle here? It will make it easier to understand and avoid wrong
> changes in the future.

Fixed!

review: Needs Resubmitting

[Riccardo Ferrazzo (f-riccardo87)]

review: Approve

[Riccardo Ferrazzo (f-riccardo87)]

review: Abstain

[Riccardo Ferrazzo (f-riccardo87)]

Excuse me, i maded some mistakes with my smartphone browser

[lp:~f-riccardo87/ubuntu-calculator-app/new-formula-structure]

48. By Riccardo Ferrazzo on 2013-04-12

merged autopilot tests

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:48
http://91.189.93.125:8080/job/ubuntu-calculator-app-ci/11/
Executed test runs:
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-calculator-app-quantal-amd64-ci/11
    SUCCESS: http://91.189.93.125:8080/job/ubuntu-calculator-app-raring-amd64-ci/11

Click here to trigger a rebuild:
http://91.189.93.125:8080/job/ubuntu-calculator-app-ci/11/rebuild

review: Approve (continuous-integration)

[Gustavo Pichorim Boiko (boiko)]

Looks good now!

review: Approve

=== modified file 'Simple/CalcKeyboard.qml'
--- Simple/CalcKeyboard.qml	2013-04-07 11:18:00 +0000
+++ Simple/CalcKeyboard.qml	2013-04-12 09:09:27 +0000
@@ -33,7 +33,7 @@
             y: 0
             text: "C"
             onClicked: {
-                pop();
+                formulaPop();
             }
         }
 
@@ -50,14 +50,14 @@
             x: (calcGridUnit*11)*2
             y: 0
             text: "÷"
-            onClicked: formulaPush(text)
+            onClicked: formulaPush("/")
         }
 
         KeyboardButton {
             x: (calcGridUnit*11)*3
             y: 0
             text: "×"
-            onClicked: formulaPush(text)
+            onClicked: formulaPush("*")
         }
 
         KeyboardButton {
@@ -85,7 +85,7 @@
             x: (calcGridUnit*11)*3
             y: (calcGridUnit*8)
             text: "−"
-            onClicked: formulaPush(text)
+            onClicked: formulaPush('-')
         }
 
         KeyboardButton {

=== modified file 'Simple/CalcLabel.qml'
--- Simple/CalcLabel.qml	2013-03-19 17:52:36 +0000
+++ Simple/CalcLabel.qml	2013-04-12 09:09:27 +0000
@@ -14,6 +14,7 @@
     property string labelText: ""
     property int numbersHeight: units.gu(4)
     property string numbersColor: '#757373'
+    property bool isLast: false
 
     Row{
         id: row
@@ -39,6 +40,12 @@
         }
         Label {
             id: formulaLabel
+
+            // this property is used by autopilot tests to find the correct item
+            // FIXME: implement it in a better way
+            property bool isLast: root.isLast
+
+            objectName: "formulaLabel"
             width: units.gu(25)
             color: numbersColor
             anchors.bottom: parent.bottom

=== modified file 'Simple/Screen.qml'
--- Simple/Screen.qml	2013-03-20 16:28:57 +0000
+++ Simple/Screen.qml	2013-04-12 09:09:27 +0000
@@ -77,6 +77,7 @@
                         operation: _operation
                         numbersColor: (isLastItem && index === repeater.model.count-1) ? "#dd4814" : "#757373"
                         numbersHeight: (isLastItem && index === repeater.model.count-1) ? units.gu(7) : units.gu(4)
+                        isLast: (isLastItem && index === repeater.model.count-1)
 
                         onLabelTextChanged: {
                             root.labelTextUpdated(index, labelText)

=== modified file 'Simple/SimplePage.qml'
--- Simple/SimplePage.qml	2013-04-06 08:47:15 +0000
+++ Simple/SimplePage.qml	2013-04-12 09:09:27 +0000
@@ -1,168 +1,59 @@
 import QtQuick 2.0
 import Ubuntu.Components 0.1
 import "../engine.js" as CALC
-//TODO: check for unneeded scanner initializations
+import "../formula.js" as F
 
 Page {
 
-    property var context: new CALC.Context;
-    property var scanner: new CALC.Scanner(engineFormula.join(''), context);
+    property var formula: new F.Formula()
     property var screenFormula: [{_text:'', _operation: '', _number:''}]
-    property var engineFormula: ['']
 
     function formulaPush(visual) {
-        var engine;
-        switch(visual){
-        case '÷':
-            engine = '/';
-            break;
-        case '×':
-            engine = '*';
-            break;
-        case '−':
-            engine = '-';
-            break;
-        default:
-            engine = visual;
-        }
-        if(screenFormula.length === 0){
-            screenFormula.push({_text:'', _operation: '', _number:''});
-        }
-        if(engineFormula.length === 0){
-            engineFormula.push('');
-        }
-        if(screenFormula[screenFormula.length-1]._operation === "=") return;
         try{
-            var n_scanner = new CALC.Scanner(engineFormula.join('') + engine, context);
-        } catch(exception) {
-            console.log("errore",exception)
+        var last = formula.push(visual);
+        } catch(exception){
             return;
         }
-        if(n_scanner.tokens.length > 1){
-            switch(n_scanner.tokens.last().type){
-            case CALC.T_PLUS:
-            case CALC.T_MINUS:
-            case CALC.T_DIV:
-            case CALC.T_TIMES:
-                if(n_scanner.tokens[n_scanner.tokens.length-2].type & CALC.T_OPERATOR) return;
-                scanner = n_scanner;
-                screenFormula.push({_text:'', _operation: '', _number:''});
-                screenFormula[screenFormula.length-1]._operation = visual;
-                engineFormula.push('');
-                engineFormula[engineFormula.length-1] += engine;
-                formulaView.currentOperatorsChanged();
-                return;
-            case CALC.T_UNARY_PLUS:
-            case CALC.T_UNARY_MINUS:
-                if(n_scanner.tokens[n_scanner.tokens.length-2].type === CALC.T_UNARY_MINUS ||
-                        n_scanner.tokens[n_scanner.tokens.length-2].type === CALC.T_UNARY_PLUS ) return;
-                break;
-            }
+
+        //if the element to add is a number or an unary operator it is added to the current line
+        //otherwise the element is a binary operator and it needs to be added to a new line
+        if(last.type === CALC.T_NUMBER || last.type === CALC.T_UNARY_MINUS){
+            screenFormula[screenFormula.length - 1]._number += last.value.toString();
+        } else {
+            screenFormula.push({_text:'', _operation: last.value.toString(), _number: ''})
         }
-        scanner = n_scanner;
-        screenFormula[screenFormula.length-1]._number += visual;
-        engineFormula[engineFormula.length-1] += engine;
         formulaView.currentOperatorsChanged();
     }
 
     function changeSign(){
-        if(screenFormula[screenFormula.length-1]._operation === "=") return;
-        var prev_tokens = [];
-        var add_minus = true;
+        formula.changeSign();
+        formulaView.currentOperatorsChanged();
+    }
+
+    function calculate() {
+        try{
+            var result = formula.calculate();
+        } catch(exception) {
+            formulaPop();
+            calculate();
+        }
+        screenFormula.push({_text:'', _operation: '=', _number: result.toString()});
+        formulaView.currentOperatorsChanged();
+    }
+
+    function formulaPop(){
+        if(screenFormula.length <= 1){
+            clear();
+            return;
+        }
+        formula.pop();
         screenFormula.pop();
-        engineFormula.pop();
-        if(scanner.tokens.length === 0){
-            prev_tokens.push(new CALC.Token('-', CALC.T_UNARY_MINUS));
-        }
-        while(scanner.tokens.length > 0 ){
-            var last = scanner.tokens.pop();
-            if(last.type & CALC.T_OPERATOR &&
-                    last.type !== CALC.T_UNARY_MINUS &&
-                    last.type !== CALC.T_UNARY_PLUS ){
-                if(add_minus){
-                    prev_tokens.push(new CALC.Token('-', CALC.T_UNARY_MINUS));
-                }
-                prev_tokens.push(last);
-                break;
-            }
-            if(last.type === CALC.T_UNARY_MINUS){
-                add_minus = false;
-                continue;
-            }
-            prev_tokens.push(last);
-            if(scanner.tokens.length === 0 && add_minus){
-                prev_tokens.push(new CALC.Token('-', CALC.T_UNARY_MINUS));
-            }
-        }
-        while(prev_tokens.length > 0){
-            var last = prev_tokens.pop();
-            var visual;
-            switch(last.type){
-            case CALC.T_TIMES:
-                visual = '×';
-                break;
-            case CALC.T_DIV:
-                visual = '÷';
-                break;
-            case CALC.T_MINUS:
-                visual = "−";
-                break;
-            default:
-                visual = last.value.toString();
-            }
-            formulaPush(visual);
-        }
-        formulaView.currentOperatorsChanged();
-    }
-
-    function calculate() {
-        if(screenFormula[screenFormula.length-1]._operation === "="){
-            var lastOperation = engineFormula.pop();
-            var previousResult = screenFormula[screenFormula.length-1]._number
-            engineFormula = [previousResult, lastOperation];
-        }
-        try{
-            scanner = new CALC.Scanner(engineFormula.join(""), context);
-            var result = new CALC.Parser(scanner).reduce(context);
-            if (result === Number.POSITIVE_INFINITY)
-                result = '∞';
-            else if (result === Number.NEGATIVE_INFINITY)
-                result = '-∞';
-        } catch(exception) {
-            if(exception instanceof CALC.DivisionByZeroError){
-                result = "division by zero error";
-            } else{
-                screenFormula.pop();
-                engineFormula.pop();
-                scanner = new CALC.Scanner(engineFormula.join(''), context);
-                calculate();
-                return;
-            }
-        }
-        screenFormula.push({_text:'', _operation: '=', _number:result.toString()});
-        formulaView.currentOperatorsChanged();
-    }
-
-    function pop(){
-        if(screenFormula.length === 0) return;
-        var lastRow = screenFormula.pop();
-        formulaView.currentOperatorsChanged();
-        if(screenFormula[screenFormula.length-1]._operation === "=") return;
-        var oldFormula = screenFormula;
-        clear();
-        for(var i = 0; i<oldFormula.length; i++){
-            if(oldFormula[i]._operation !== '')
-                formulaPush(oldFormula[i]._operation);
-            formulaPush(oldFormula[i]._number);
-            screenFormula[screenFormula.length-1]._text = oldFormula[i]._text;
-        }
         formulaView.currentOperatorsChanged();
     }
 
     function clear(){
         screenFormula = [{_text:'', _operation: '', _number:''}];
-        engineFormula = [''];
-        scanner = new CALC.Scanner(engineFormula.join(''), context);
+        formula = new F.Formula();
         formulaView.currentOperatorsChanged();
     }
 
@@ -173,7 +64,7 @@
             return;
         }
         if(temp.tokens.length === 1 && temp.tokens.last().type === CALC.T_NUMBER){
-            formulaPush(numberToAdd, numberToAdd);
+            formulaPush(numberToAdd);
         }
     }
 

=== added file 'formula.js'
--- formula.js	1970-01-01 00:00:00 +0000
+++ formula.js	2013-04-12 09:09:27 +0000
@@ -0,0 +1,133 @@
+Qt.include("engine.js");
+
+var context = new Context;
+
+function Formula() {
+    var previous = new Token(0, T_NUMBER);
+    var formula = '';
+    var results = [];
+    var lookupTbl = {
+        '-': '−',
+        '/': '÷',
+        '*': '×'
+    };
+
+    var _calculate = function(func){
+        try{
+            var result = parse(func, context);
+            if (result === Number.POSITIVE_INFINITY)
+                result = '∞';
+            else if (result === Number.NEGATIVE_INFINITY)
+                result = '-∞';
+        }catch(exception){
+            if(exception instanceof DivisionByZeroError){
+                result = "division by zero error";
+            } else{
+                throw new SyntaxError("malformed calculation");
+            }
+        }
+        return result;
+    }
+
+    this.push = function(digit){
+        if(results.length > 0) throw new SyntaxError("cannot push");
+        var last = (new Scanner(previous.value.toString()+digit, context)).tokens.pop();
+        switch(last.type){
+        case T_PLUS:
+        case T_MINUS:
+        case T_DIV:
+        case T_TIMES:
+            if(previous.type & T_OPERATOR) return;
+            break;
+        case T_UNARY_PLUS:
+        case T_UNARY_MINUS:
+            if(previous.type === T_UNARY_MINUS || previous.type === T_UNARY_PLUS ) return;
+            break;
+        }
+        formula += digit;
+        previous = last;
+        var ret = {value: '', type: last.type};
+        if(lookupTbl[digit] !== undefined){
+            ret.value = lookupTbl[digit];
+        } else {
+            ret.value = digit;
+        }
+        return ret;
+    };
+
+    this.pop = function(){
+        if(results.length > 0){
+            results.pop();
+            return;
+        }
+        var scannedFormula = new Scanner(formula, context);
+        var last = {type:T_NUMBER};
+        var removeSize = 0;
+        while(scannedFormula.tokens.length > 0 && (last.type === T_NUMBER || last.type === T_UNARY_PLUS || last.type === T_UNARY_MINUS)){
+            last = scannedFormula.tokens.pop();
+            removeSize += last.value.toString().length
+        }
+        formula = formula.substring(0, formula.length - removeSize);
+        previous = scannedFormula.tokens.pop();
+    };
+
+    this.calculate = function(){
+        var result;
+        var previousSize = previous.value.toString().length;
+        if(formula.length <= previousSize) return;
+        if(results.length > 0){
+            var scannedFormula = new Scanner(formula, context);
+            var last = scannedFormula.tokens.pop();
+            var newFormula = last.value.toString();
+            while(last.type !== T_PLUS &&
+                  last.type !== T_MINUS &&
+                  last.type !== T_DIV &&
+                  last.type !== T_TIMES ){
+                last = scannedFormula.tokens.pop();
+                newFormula = last.value.toString() + newFormula;
+            }
+            result = _calculate(results[results.length-1]+newFormula);
+        } else {
+            result = _calculate(formula);
+        }
+        results.push(result.toString());
+        return result;
+    }
+
+    this.changeSign = function(){
+        if(results.length > 0) throw new SyntaxError("cannot edit formula");
+        switch(previous.type){
+        case T_PLUS:
+        case T_MINUS:
+        case T_DIV:
+        case T_TIMES:
+            formulaPush("-");
+            break;
+        default:
+            var scannedFormula = new Scanner(formula, context);
+            var last = {type: undefined};
+            var toAdd = [];
+            var addMinus = true;
+            while(scannedFormula.tokens.length > 0 && last.type !== T_PLUS &&
+                  last.type !== T_MINUS &&
+                  last.type !== T_DIV &&
+                  last.type !== T_TIMES ){
+                last = scannedFormula.tokens.pop();
+                if(last.type !== T_UNARY_MINUS && last.type !== T_UNARY_PLUS){
+                    toAdd.push(last);
+                } else {
+                    if(last.type === T_UNARY_MINUS)
+                        addMinus = false;
+                }
+            }
+            formulaPop();
+            while(toAdd.length > 0){
+                var el = toAdd.pop()
+                formulaPush(el.value.toString());
+                if((el.type & T_OPERATOR) && addMinus){
+                    formulaPush('-');
+                }
+            }
+        }
+    }
+}

=== renamed file 'tests/autopilot/ubuntu_calculator_app/emulators/main_buttons.py' => 'tests/autopilot/ubuntu_calculator_app/emulators/simple_page.py'
--- tests/autopilot/ubuntu_calculator_app/emulators/main_buttons.py	2013-03-22 12:15:46 +0000
+++ tests/autopilot/ubuntu_calculator_app/emulators/simple_page.py	2013-04-12 09:09:27 +0000
@@ -4,3 +4,15 @@
 # This program is free software: you can redistribute it and/or modify it
 # under the terms of the GNU General Public License version 3, as published
 # by the Free Software Foundation.
+
+class SimplePage(object):
+    """An emulator class that makes it easy to interact with the call panel."""
+    def __init__(self, app):
+        self.app = app
+
+    def get_button(self, text):
+        return self.app.select_single('KeyboardButton', text=text)
+
+    def get_formula_label(self):
+        # FIXME: find a better way to find the latest formula label
+        return self.app.select_single('Label', objectName='formulaLabel', isLast=True)

=== modified file 'tests/autopilot/ubuntu_calculator_app/tests/__init__.py'
--- tests/autopilot/ubuntu_calculator_app/tests/__init__.py	2013-03-22 12:15:46 +0000
+++ tests/autopilot/ubuntu_calculator_app/tests/__init__.py	2013-04-12 09:09:27 +0000
@@ -17,22 +17,22 @@
 
 class CalculatorTestCase(AutopilotTestCase, QtIntrospectionTestMixin):
 
-	"""A common testcase class that provides useful methods for calculator app."""
-
-	def setUp(self):
-		super(CalculatorTestCase, self).setUp()
-
-		if os.path.realpath(__file__).startswith("/usr/"):
+    """A common testcase class that provides useful methods for calculator app."""
+
+    def setUp(self):
+        super(CalculatorTestCase, self).setUp()
+
+        if os.path.realpath(__file__).startswith("/usr/"):
             self.launch_test_installed()
         else:
             self.launch_test_local()
 
     def launch_test_local(self):
-    	self.app = self.launch_test_application(
-    		"qmlscene", "../../ubuntu-calculator-app.qml"
-    		)
+        self.app = self.launch_test_application(
+            "qmlscene", "../../ubuntu-calculator-app.qml"
+            )
 
     def launch_test_installed(self):
-    	self.app = self.launch_test_application(
-    		"ubuntu-calculator-app"
-    		)
+        self.app = self.launch_test_application(
+            "ubuntu-calculator-app"
+            )

=== renamed file 'tests/autopilot/ubuntu_calculator_app/tests/test_main_buttons.py' => 'tests/autopilot/ubuntu_calculator_app/tests/test_simple_page.py'
--- tests/autopilot/ubuntu_calculator_app/tests/test_main_buttons.py	2013-03-22 14:17:43 +0000
+++ tests/autopilot/ubuntu_calculator_app/tests/test_simple_page.py	2013-04-12 09:09:27 +0000
@@ -14,16 +14,84 @@
 from autopilot.matchers import Eventually
 
 from ubuntu_calculator_app.tests import CalculatorTestCase
-
-
-class TestMainButtons(CalculatorTestCase):
-
-	def setUp(self):
-		super(TestMainButtons, self).setUp()
-
-	def test_this(self):
-		buttons = self.app.select_many("KeyboardButton")
-
-		for keys in buttons:
-			self.pointing_device.move_to_object(keys)
-			self.pointing_device.click()
+from ubuntu_calculator_app.emulators.simple_page import SimplePage
+
+class TestSimplePage(CalculatorTestCase):
+
+    def setUp(self):
+        super(TestSimplePage, self).setUp()
+
+    def test_this(self):
+        buttons = self.app.select_many("KeyboardButton")
+
+        for keys in buttons:
+            self.pointing_device.move_to_object(keys)
+            self.pointing_device.click()
+
+    def test_operation_after_clear(self):
+        """ Test that after clearing one calculation, the next is still correct (bug #1164973) """
+        oneButton = self.simple_page.get_button("1")
+        plusButton = self.simple_page.get_button("+")
+        equalsButton = self.simple_page.get_button("=")
+        clearButton = self.simple_page.get_button("C")
+
+        # type 1+1=
+        self.pointing_device.move_to_object(oneButton)
+        self.pointing_device.click()
+        self.pointing_device.move_to_object(plusButton)
+        self.pointing_device.click()
+        self.pointing_device.move_to_object(oneButton)
+        self.pointing_device.click()
+        self.pointing_device.move_to_object(equalsButton)
+        self.pointing_device.click()
+
+        # make sure the result is already there
+        self.assertThat(self.simple_page.get_formula_label().text, Eventually(Equals("2")))
+
+        # now clear the input by pressing "C" three times
+        self.pointing_device.move_to_object(clearButton)
+        self.pointing_device.click()
+        self.pointing_device.click()
+        self.pointing_device.click()
+
+        # and do the operation again to make sure the result is correct
+        self.pointing_device.move_to_object(oneButton)
+        self.pointing_device.click()
+        self.pointing_device.move_to_object(plusButton)
+        self.pointing_device.click()
+        self.pointing_device.move_to_object(oneButton)
+        self.pointing_device.click()
+        self.pointing_device.move_to_object(equalsButton)
+        self.pointing_device.click()
+
+        # make sure the result is correct
+        self.assertThat(self.simple_page.get_formula_label().text, Eventually(Equals("2")))
+
+    def test_equals_dont_change_numbers(self):
+        """ Test that typing a number and pressing "=" won't change the number (bug #1165894) """
+        oneButton = self.simple_page.get_button("1")
+        twoButton = self.simple_page.get_button("2")
+        threeButton = self.simple_page.get_button("3")
+        equalsButton = self.simple_page.get_button("=")
+
+        # type the initial number
+        self.pointing_device.move_to_object(oneButton)
+        self.pointing_device.click()
+        self.pointing_device.move_to_object(twoButton)
+        self.pointing_device.click()
+        self.pointing_device.move_to_object(threeButton)
+        self.pointing_device.click()
+
+        # check that the label is displaying correctly
+        self.assertThat(self.simple_page.get_formula_label().text, Eventually(Equals("123")))
+
+        # click the "=" button
+        self.pointing_device.move_to_object(equalsButton)
+        self.pointing_device.click()
+
+        # check that the label is still displaying correctly
+        self.assertThat(self.simple_page.get_formula_label().text, Eventually(Equals("123")))
+
+    @property
+    def simple_page(self):
+        return SimplePage(self.app)