Android Question A simple calculator closes down

U

username_987

Member
Hello,

In attached project, it's a simple calculator, I got it to work, however, when pressing any sign (like + or -) for more than once, the app closes

I believe the problem could be in (docalc) sub, the following:
B4X: 
Sub docalc (math As String,newmath As String)
    If math = "plus" Then
        total = nr1 + nr2
    Else If math = "minus" Then
        total = nr1 - nr2
    Else If math = "mul" Then
        total = nr1 * nr2
    Else If math = "div" Then
        total = nr1 / nr2
    Else If math = "percent" Then
        total = nr1 / 100
    Else
        Return
    End If
    If newmath <> "" Then
        mathstr = newmath
        nr2 = ""
    Else
        mathstr = ""
        nr2 = ""
    End If
    nr1 = total
    totallb1.Text = NumberFormat(total, 0, 30) & space
End Sub

Thank you for helping with it
 

Attachments

  • calculator debug.zip
    10.7 KB · Views: 35
Sort by date Sort by votes
B

Brian Dean

Well-Known Member
Licensed User
Longtime User
I added this line to your code in Sub docalc :

B4X: 
Sub docalc (math As String,newmath As String)
    Log("nr1 = " & nr1 & " ; nr2 = " & nr2)   ' <===

If you tap any sign key twice then you will see that Sub docalc is called, but nr2 is empty (blank). That is what causes the crash. You will need to detect the situation where an operator key is pressed twice in succession and prevent Sub docalc being called, or you could check in "docalc" that neither of the arguments are empty.

Have you tried running in "Debug" mode? That helps you locate problems like this more quickly. Also your screen layout needs to be more adaptable; on my phone (Pixel 4a) the operator keypads are only just visible at the edge of the screen in portrait mode.
 
Upvote 0
U

username_987

Member
Well that's great, it works now after excluding nr2 empty case

However, when I press + + -, that gives a +, or - - +, that gives a -, despite pressing - or + lastly

I think that's the last thing before the app works in full, I used a calculator example from the forum and modified it since it had a number of issues

Regarding the layout, maybe because I recreated the app to display here, I mistakenly left the (variants) at a low resolution, so it displays large in phone screen

Thank you really
 
Upvote 0
U

username_987

Member
I added an exclusion to when nr2 = "", then Return

Not sure if it is related to the current issue, pressing a sign more than once causes it to be used despite which sign is pressed after it
 
Upvote 0
U

username_987

Member
Reminder: the problem is actually pressing more than a math sign, the first pressed one will be used always

The example that I've modified is attached

In the example it was:
B4X: 
Select btntag
            Case "plus"
                If mathstr = btntag Then Return
                If mathstr = "" Or mathstr <> btntag Then mathstr = btntag Else docalc(mathstr,btntag) 'if mathstr is empty set it to selected buttons else do calculation
            Case "minus"
                If mathstr = btntag Then Return
                If mathstr = "" Or mathstr <> btntag Then mathstr = btntag Else docalc(mathstr,btntag)
            Case "mul"
                If mathstr = btntag Then Return
                If mathstr = "" Or mathstr <> btntag Then mathstr = btntag Else docalc(mathstr,btntag)
            Case "div"
                If mathstr = btntag Then Return
                If mathstr = "" Or mathstr <> btntag Then mathstr = btntag Else docalc(mathstr,btntag)

Yet that yields when pressing (1+2) then pressing a sign like (-), the result will not be shown, plus that when you press an additional number after the second math sign, it will be added to the current number (like if 1 is displayed, and you press 2, it will be 12, if 2 is displayed and you press 10, it will be 210)
 

Attachments

  • t1calc.zip
    3.7 KB · Views: 38
Upvote 0
B

Brian Dean

Well-Known Member
Licensed User
Longtime User
Look closely at the code in post #7. The same lines of code are repeated four times. Repeated code is a clear fingerprint of poor coding and what you have here is a very bad example. If you are trying to learn how to code then you have chosen the wrong model to learn from.

"the problem is actually pressing more than a math sign, the first pressed one will be used always"
Click to expand...

Look at this line of code from your current example :

B4X: 
  If mathstr = "" Then mathstr = btntag Else docalc(mathstr,btntag)

This says "If <mathstr> has not been set then set it to the new value (+, -, *, /) but if it has been set then start the calculation using the original <mathstr> value". However in subroutine <docalc> the calculation will not be done as <nr2> has not yet been set. So, once <mathstr> has been set any second arithmetic value will be ignored and the original one will remain in place, as you describe.

Now look at the original code :

B4X: 
  If mathstr = btntag Then Return
  If mathstr = "" Or mathstr <> btntag Then mathstr = btntag Else docalc(mathstr,btntag)

This says "If the new <btntag> value matches a previously set value in <mathstr> then do nothing. Otherwise if <mathstr> has not been set or it has been set to a different <btntag> value then replace it with the new value. If <mathstr> has not been set and it is not the same as <btntag> then proceed with the calculation". The first part of this is fine, but the rest is a puzzle to me - I cannot see any situation in which the "Else" clause would be triggered, or why you would want it to be.

I have tried running the original code and, as you say, it does not work as you might expect if two operator keys are pressed in sequence. It is has many errors in its design. I have tried to find a simple correction that will fix the problem and always other problems appear. My advice to you is you to look for a better example - this is not a good place to start. I would also add that there is no such thing as the "simple calculator" application that you mention in your first post - coding a good calculator is in fact quite difficult.
 
Upvote 0
U

username_987

Member
Brian Dean said:
Look closely at the code in post #7. The same lines of code are repeated four times. Repeated code is a clear fingerprint of poor coding and what you have here is a very bad example. If you are trying to learn how to code then you have chosen the wrong model to learn from.



Look at this line of code from your current example :

B4X: 
  If mathstr = "" Then mathstr = btntag Else docalc(mathstr,btntag)

This says "If <mathstr> has not been set then set it to the new value (+, -, *, /) but if it has been set then start the calculation using the original <mathstr> value". However in subroutine <docalc> the calculation will not be done as <nr2> has not yet been set. So, once <mathstr> has been set any second arithmetic value will be ignored and the original one will remain in place, as you describe.

Now look at the original code :

B4X: 
  If mathstr = btntag Then Return
  If mathstr = "" Or mathstr <> btntag Then mathstr = btntag Else docalc(mathstr,btntag)

This says "If the new <btntag> value matches a previously set value in <mathstr> then do nothing. Otherwise if <mathstr> has not been set or it has been set to a different <btntag> value then replace it with the new value. If <mathstr> has not been set and it is not the same as <btntag> then proceed with the calculation". The first part of this is fine, but the rest is a puzzle to me - I cannot see any situation in which the "Else" clause would be triggered, or why you would want it to be.

I have tried running the original code and, as you say, it does not work as you might expect if two operator keys are pressed in sequence. It is has many errors in its design. I have tried to find a simple correction that will fix the problem and always other problems appear. My advice to you is you to look for a better example - this is not a good place to start. I would also add that there is no such thing as the "simple calculator" application that you mention in your first post - coding a good calculator is in fact quite difficult.
Click to expand...
I agree with you in everything. And this will be useful in the forum as a completed calculator example, once it's finished, if it did

Thank you for your analyzing to code

So, now we need to fix (two signs pressed in sequence issue), in following code:


B4X: 
Sub Process_Globals
    'These global variables will be declared once when the application starts.
    'These variables can be accessed from all modules.
    Private xui As XUI
End Sub

Sub Globals
    'These global variables will be redeclared each time the activity is created.
    Private nrbtndiv As Button
    Private cl1 As Panel
    Private nrbtn0 As Button
    Private nrbtn1 As Button
    Private nrbtn2 As Button
    Private nrbtn3 As Button
    Private nrbtn4 As Button
    Private nrbtn5 As Button
    Private nrbtn6 As Button
    Private nrbtn7 As Button
    Private nrbtn8 As Button
    Private nrbtn9 As Button
    Private nrbtnac As Button
    Private nrbtndot As Button
    Private nrbtneq As Button
    Private nrbtnminus As Button
    Private nrbtnmul As Button
    Private nrbtnplus As Button
    Private nrbtnpercent As Button
    Dim nr1,nr2 As String  = ""
    Dim total As String  = ""
    Dim mathstr As String = ""
    Dim space As String = " "
    Private totallb1 As EditText
End Sub

Sub Activity_Create(FirstTime As Boolean)
    Activity.LoadLayout("Layout")
End Sub

Sub Activity_Resume

End Sub

Sub Activity_Pause (UserClosed As Boolean)

End Sub

Sub Button1_Click

End Sub


Sub nrbtn_Click
    Dim btn As Button = Sender
    Dim btntag As String = btn.Tag
    If IsNumber(btntag) Then
        If mathstr <> "" Then
            If nr2.Length = 1 Then
                If nr2 = "0" And btntag = "0" Then
                    nr2 = "0"
                Else
                    If nr2 = "0" And btntag <> "0" Then nr2 = btntag Else nr2 = nr2 & btntag
                End If
            Else
                nr2 = nr2 & btntag
            End If
            totallb1.Text = nr2 & space
        Else
            If nr1.Length = 1 Then
                If nr1 = "0" And btntag = "0" Then
                    nr1 = "0"
                Else
                    If nr1 = "0" And btntag <> "0" Then nr1 = btntag Else nr1 = nr1 & btntag
                End If
            Else
                nr1 = nr1 & btntag
            End If
            totallb1.Text = nr1 & space
        End If
    Else
        If nr1 = "" Then Return
        Select btntag
            Case "plus"
                If mathstr = "" Then mathstr = btntag Else docalc(mathstr,btntag) 'if mathstr is empty set it to selected buttons else do calculation
            Case "minus"
                If mathstr = "" Then mathstr = btntag Else docalc(mathstr,btntag)
            Case "mul"
                If mathstr = "" Then mathstr = btntag Else docalc(mathstr,btntag)
            Case "div"
                If mathstr = "" Then mathstr = btntag Else docalc(mathstr,btntag)
            Case "percent"
                If mathstr = "" Then mathstr = btntag Else docalc(mathstr,btntag)
                docalc(mathstr,"")
                
            Case "eq"
                If nr2 = "" Then Return
                docalc(mathstr,"")
            Case "AC"
                totallb1.Text = "0" & space
                nr1 = ""
                nr2 = ""
                mathstr = ""
                total = ""
            Case "."
                If mathstr <> "" Then
                    If Not(nr2.Contains(".")) Then nr2 = nr2 & "."
                    totallb1.Text = nr2 & space
                Else
                    If nr1 = "" And totallb1.Text = "0" Then nr1 = "0"
                    If Not(nr1.Contains(".")) Then nr1 = nr1 & "."
                    totallb1.Text = nr1 & space
                End If
        End Select
    End If
End Sub

Sub docalc (math As String,newmath As String)
    If nr2 = "" Then
    Return
    End If
    If math = "plus" Then
        total = nr1 + nr2
    Else If math = "minus" Then
        total = nr1 - nr2
    Else If math = "mul" Then
        total = nr1 * nr2
    Else If math = "div" Then
        total = nr1 / nr2
    Else If math = "percent" Then
        total = nr1 / 100
    Else
        Return
    End If
    If newmath <> "" Then
        mathstr = newmath
        nr2 = ""
    Else
        mathstr = ""
        nr2 = ""
    End If
    nr1 = total
    totallb1.Text = NumberFormat(total, 0, 30) & space
End Sub
 
Upvote 0
U

username_987

Member
A calculator example in VB is widely available on google, like:
www.sourcecodester.com

Simple Calculator in (VB) Visual Basic with Source Code

This is a Sample calculator that I created in visual basic 2008, and this sample will be more useful for learners who are interested in VB 2008 and fortunately the VS 2019 can upgrade the source code perfectly. You can study it and modify it as more as you like.
www.sourcecodester.com www.sourcecodester.com

And I'm still unable to get the (pressing more than one sign) issue to be fixed

Thank you
 
Upvote 0
U

username_987

Member
I got it to work, seems free of problems to me, using the following code:
B4X: 
Sub nrbtn_Click
    Dim btn As Button = Sender
    Dim btntag As String = btn.Tag
    If IsNumber(btntag) Then
        If mathstr <> "" Then
            If nr2.Length = 1 Then
                If nr2 = "0" And btntag = "0" Then
                    nr2 = "0"
                Else
                    If nr2 = "0" And btntag <> "0" Then nr2 = btntag Else nr2 = nr2 & btntag
                End If
            Else
                nr2 = nr2 & btntag
            End If
            totallb1.Text = nr2 & space
        Else
            If nr1.Length = 1 Then
                If nr1 = "0" And btntag = "0" Then
                    nr1 = "0"
                Else
                    If nr1 = "0" And btntag <> "0" Then nr1 = btntag Else nr1 = nr1 & btntag
                End If
            Else
                nr1 = nr1 & btntag
            End If
            totallb1.Text = nr1 & space
        End If
    Else
        If nr1 = "" Then Return
        Select btntag
            Case "plus"
                If mathstr = "" Then
                     mathstr = btntag
                    Else
                docalc(mathstr,btntag) 'if mathstr is empty set it to selected buttons else do calculation
                mathstr = btntag
                End If
            Case "minus"
                If mathstr = "" Then
                    mathstr = btntag
                    Else
                        docalc(mathstr,btntag)
                        mathstr = btntag
                        End If
            Case "mul"
                If mathstr = "" Then
                    mathstr = btntag
                Else
                    docalc(mathstr,btntag)
                    mathstr = btntag
                End If
            Case "div"
                If mathstr = "" Then
                    mathstr = btntag
                Else
                    docalc(mathstr,btntag)
                    mathstr = btntag
                End If
            Case "percent"
                If mathstr = "" Then
                    mathstr = btntag
                Else
                    docalc(mathstr,btntag)
                    mathstr = btntag
                End If
                docalc(mathstr,"")   
            Case "eq"
                If nr2 = "" Then Return
                docalc(mathstr,btntag)
                mathstr = ""
            Case "AC"
                totallb1.Text = "0" & space
                nr1 = ""
                nr2 = ""
                mathstr = ""
                total = ""
            Case "."
                If mathstr <> "" Then
                    If Not(nr2.Contains(".")) Then nr2 = nr2 & "."
                    totallb1.Text = nr2 & space
                Else
                    If nr1 = "" And totallb1.Text = "0" Then nr1 = "0"
                    If Not(nr1.Contains(".")) Then nr1 = nr1 & "."
                    totallb1.Text = nr1 & space
                End If
        End Select
    End If
End Sub

Sub docalc (math As String,newmath As String)
    If nr2 = "" Then
        nr2 = "0"
    End If
    If math = "plus" Then
        total = nr1 + nr2
    Else If math = "minus" Then
        total = nr1 - nr2
    Else If math = "mul" Then
        total = nr1 * nr2
    Else If math = "div" Then
        total = nr1 / nr2
    Else If math = "percent" Then
        total = nr1 / 100
    Else
        Return
    End If
    If newmath <> "" Then
        mathstr = newmath
        nr2 = ""
    Else
        mathstr = ""
        nr2 = ""
    End If
    nr1 = total
    totallb1.Text = NumberFormat(total, 0, 30) & space
End Sub

Thanks really to everyone for your useful comments
 
Upvote 0
U

username_987

Member
Using the code at last post, pressing / or X repeatedly, yields (infinity) and (zero) in order

So, I used the following code, and now the calculator is free from any sort of problems

B4X: 
Sub docalc (math As String,newmath As String)
    If math = "plus" Then
        If nr2 = "" Then
            nr2 = "0"
        End If
        total = nr1 + nr2
    Else If math = "minus" Then
        If nr2 = "" Then
            nr2 = "0"
        End If
        total = nr1 - nr2
    Else If math = "mul" Then
        If nr2 = "" Then
            nr2 = "1"
        End If
        total = nr1 * nr2
    Else If math = "div" Then
        If nr2 = "" Then
            nr2 = "1"
        End If
        total = nr1 / nr2
    Else If math = "percent" Then
        total = nr1 / 100

Thanks to everyone
 
Upvote 0
U

username_987

Member
Just a last problem with the code in last post above: when you do a calculation and press (equals), then you get the result in screen, if you pressed a number after it, the number adds to the digits in screen, so the screen doesn't reset to that number (the correct supposed thing)

So, if you press: 1+2= (which is = 3)
Then you press: 1, you will see in screen 31

However, if you press after the = , you press + 1, you will see correct result in screen which is 4

The same is with % sign

I just want to see if anyone is able to solve this problem, it can be left so, but is better to be solved for a 100% problems free calculator

Thanks
 
Upvote 0
You must log in or register to reply here.

Similar Threads

Terradrones
  • Question
Android Question Simple CAD
Replies
5
Views
833
amorosik
amorosik
max123
  • Article
Android Code Snippet [algorithm] First post. Map a value from one range to another range. The simple way.
Replies
0
Views
3K
max123
max123
Erel
  • Locked
  • Article
B4A Library [B4X] XUI Views - Cross platform views and dialogs
3 4 5
Replies
97
Views
193K
Erel
Erel
Johan Schoeman
  • Article
B4A Library RPN Calculator
Replies
1
Views
4K
Rantor777
Rantor777
Johan Schoeman
  • Article
B4A Library ContinuableCircleCountDownView
Replies
17
Views
5K
DonManfred
DonManfred
Top