9
\$\begingroup\$

It accepts string input like '1+1' instead of num1 = 1, num2 = 2, operation = '+' or something like that.

def Numbers(var):
    return var == '0' or var == '1' or var == '2' or var == '3' or var == '4' or var == '5' or var == '6' or var == '7' or var == '8' or var == '9'

def Test4Num(varstr):
    n = 0
    var = ''
    try: 
        while Numbers(varstr[n]):
            var += varstr[n]
            n += 1
    except: pass
    return (int(var), n)

def operation(string, num1, num2):
    if string == '+':
        return num1 + num2
    if string == '-':
        return num1-num2
    if string == '*':
        return num1*num2
    if string == '/':
        return num1/num2
    if string == '^':
        return num1 ** num2

def operator(operato):
    return operato == '+' or operato == '-' or operato == '*' or operato == '/' or operato == '^'


negate = False
char = input('')

print(char + '=')
while True:
    try: 
        if char[0] == '-': #for negative numbers
            negate = True #because here the numbers are string format
            char = char[1:]
        number1 = Test4Num(char)[0]
        if negate == True:
            number1 = -number1
            negate = False
        end_number1 = Test4Num(char)[1]
        char = char[end_number1:]
        if char == '':
            print(number1)
            break
        op = char[0]
        char = char[1:]
        number2 = Test4Num(char)[0]
        end_number2 = Test4Num(char)[1]
        result = operation(op, number1, number2)
        number1 = result
        char = str(number1) + char[end_number2:]
        print(char)
    except: break
#created by Estoria Wandertag, while he was sitting on the toilet and thought about life and programming

This code is pretty straightforward, I mean it doesn't use class, any imports, etc. (ok, it can just use the 5 main operations in math, but that's extendable) and even though it's easy, I didn't find it on the internet... So that's the reason I wanted to share this. If you have ideas, please share them. I'm pretty new here and also I haven't been using Python for very long, so there are plenty of things to add, I guess. (Sorry for my awful English, I'm German)

\$\endgroup\$
6
\$\begingroup\$

On top of the other very good answers:

Code organisation and tests

It is a good idea to separate the logic about input/output from the logic computing results. In your case, you could have a function taking an expression as a string and returning a number.

This makes your code easier to understand, easier to reuse and more testable.

Also, it is a good habit to put your code actually calling the logic behind an if __name__ == "__main__": guard.

All of this taken into account, you have:

def eval_math_expr(expr):
    negate = False
    while True:
        try: 
            if expr[0] == '-': #for negative numbers
                negate = True #because here the numbers are string format
                expr = expr[1:]
            number1 = Test4Num(expr)[0]
            if negate == True:
                number1 = -number1
                negate = False
            end_number1 = Test4Num(expr)[1]
            expr = expr[end_number1:]
            if expr == '':
                return number1
            op = expr[0]
            expr = expr[1:]
            number2 = Test4Num(expr)[0]
            end_number2 = Test4Num(expr)[1]
            result = operation(op, number1, number2)
            number1 = result
            expr = str(number1) + expr[end_number2:]
        except Exception as e:
            print(e)
            break
    return number1


if __name__ == '__main__':
    interactive = False
    if interactive:
        expr = input('Enter your expression:')
        print(expr + '=')
        print(eval_math_expr(expr))
    else:
        for expr, res in {"2": 2, "2*4": 8, "4+8": 12, "100/3": 33, "2^3": 8}.items():
            result = eval_math_expr(expr)
            if res != result:
                print("Computing", expr, "got", result, "instead of", res)

I also took this chance to remove the silent catch of exceptions which makes things hard to understand when an error occurs.

By the way, bare except (with no explicit exception type caught) and ignored exceptions are usually frowned upon as they make debugging very painful.

Naming

Your function names are not very good. Your could define is_digit, get_number, perform_operation.

Simplify is_digit function

The best data structure for the check you want to perform is a set. You could write something like:

DIGITS = set('0123456789')

def is_digit(var):
    return var in DIGITS

Simplify perform_operation

The best data structure for the logic you want to perform is a dictionnary mapping operators to functions.

import operator

OPERATIONS = {
    '+' : operator.add,
    '-' : operator.sub,
    '*' : operator.mul,
    '/' : operator.floordiv,
    '^' : operator.pow,
}


def perform_operation(string, num1, num2):
    op = OPERATIONS.get(string, None)
    if op is not None:
        return op(num1, num2)
    else:
        return None  # How to handle this?

Note: using floordiv instead of truediv fixes an issue found with tests mentionned above.

Simplify get_number

In Python, you usually don't need to get elements from an object by index. What you want is usually to iterate over the different elements. I highly recommand reading/watching Ned Batchelder talk called "Loop Like A Native". In your case, you could write something like:

def get_number(varstr):
    s = ""
    for c in varstr:
        if not is_digit(c):
            break
        s += c
    return (int(s), len(s))

Using tuple unpacking

As already said, you could use number1, end_number1 = get_number(expr) to call get_number only once.

At this stage, the code looks like:

import operator

DIGITS = set('0123456789')
OPERATIONS = {
    '+' : operator.add,
    '-' : operator.sub,
    '*' : operator.mul,
    '/' : operator.floordiv,
    '^' : operator.pow,
}


def is_digit(var):
    return var in DIGITS

def get_number(varstr):
    s = ""
    for c in varstr:
        if not is_digit(c):
            break
        s += c
    return (int(s), len(s))

def perform_operation(string, num1, num2):
    op = OPERATIONS.get(string, None)
    if op is not None:
        return op(num1, num2)
    else:
        return None  # How to handle this?

def eval_math_expr(expr):
    negate = False
    while True:
        try: 
            if expr[0] == '-': #for negative numbers
                negate = True #because here the numbers are string format
                expr = expr[1:]
            number1, end_number1 = get_number(expr)
            expr = expr[end_number1:]
            if negate == True:
                number1 = -number1
                negate = False
            if expr == '':
                return number1
            op = expr[0]
            expr = expr[1:]
            number2, end_number2 = get_number(expr)
            result = perform_operation(op, number1, number2)
            number1 = result
            expr = str(number1) + expr[end_number2:]
        except Exception as e:
            print(e)
            break
    return number1


if __name__ == '__main__':
    interactive = False
    if interactive:
        expr = input('Enter your expression:')
        print(expr + '=')
        print(eval_math_expr(expr))
    else:
        for expr, res in {"2": 2, "2*4": 8, "4+8": 12, "100/3": 33, "2^3": 8, "-2": -2, "-2-3": -5}.items():
            result = eval_math_expr(expr)
            if res != result:
                print("Computing", expr, "got", result, "instead of", res)

About negate

You set negate to False at the very beginning and make sure you reset it back to False at each iteration so that the next iteration works fine. It would be clearer to initialise it to False at the beginning of the iteration. You could also set it to negate = expr[0] == '-'.

        negate = expr[0] == '-' #for negative numbers
        if negate:
            expr = expr[1:]
        number1, end_number1 = get_number(expr)
        expr = expr[end_number1:]
        if negate:
            number1 *= -1

An alterative could be to handle this as part of the get_number function.

def get_number(varstr):
    s = ""
    if varstr[0] == '-':
        s += "-"
        varstr = varstr[1:]
    for c in varstr:
        if not is_digit(c):
            break
        s += c
    return (int(s), len(s))


def eval_math_expr(expr):
    while True:
        try: 
            number1, end_number1 = get_number(expr)
            expr = expr[end_number1:]
            if expr == '':
                return number1
            op = expr[0]
            expr = expr[1:]
            number2, end_number2 = get_number(expr)
            number1 = perform_operation(op, number1, number2)
            expr = str(number1) + expr[end_number2:]
        except Exception as e:
            print(e)
            break
    return number1

The whole code is:

import operator

DIGITS = set('0123456789')
OPERATIONS = {
    '+' : operator.add,
    '-' : operator.sub,
    '*' : operator.mul,
    '/' : operator.floordiv,
    '^' : operator.pow,
}


def is_digit(var):
    return var in DIGITS

def get_number(varstr):
    s = ""
    if varstr[0] == '-':
        s += "-"
        varstr = varstr[1:]
    for c in varstr:
        if not is_digit(c):
            break
        s += c
    return (int(s), len(s))

def perform_operation(string, num1, num2):
    op = OPERATIONS.get(string, None)
    if op is not None:
        return op(num1, num2)
    else:
        return None  # How to handle this?

def eval_math_expr(expr):
    while True:
        try: 
            number1, end_number1 = get_number(expr)
            expr = expr[end_number1:]
            if expr == '':
                return number1
            op = expr[0]
            expr = expr[1:]
            number2, end_number2 = get_number(expr)
            number1 = perform_operation(op, number1, number2)
            expr = str(number1) + expr[end_number2:]
        except Exception as e:
            print(e)
            break
    return number1


if __name__ == '__main__':
    interactive = False
    if interactive:
        expr = input('Enter your expression:')
        print(expr + '=')
        print(eval_math_expr(expr))
    else:
        for expr, res in {"2": 2, "2*4": 8, "4+8": 12, "100/3": 33, "2^3": 8, "-2": -2, "-2-3": -5}.items():
            result = eval_math_expr(expr)
            if res != result:
                print("Computing", expr, "got", result, "instead of", res)

Reorganisation in the main function

At the moment, the code parses two number and an operation, computes the operation, convert the result into a string to be parsed at next iteration. Thus, each temporary result gets converted from integer to string then integer. This leads to issues when the result cannot be converted back and forth - for instance when you perform divisions with non-integer result. Also this could be optimised a lot with the following logic:

  • get the first number and store it as the current number

  • in each iteration: get the operation and the next number. Compute the operation and store the result as the current number, to be reused at next iteration.

  • at the end, return the current number.

You could write this:

def eval_math_expr(expr):
    n, end_n = get_number(expr)
    expr = expr[end_n:]
    while expr:
        op, expr = expr[0], expr[1:]
        n2, end_n = get_number(expr)
        n = perform_operation(op, n, n2)
        expr = expr[end_n:]
    return n
\$\endgroup\$
  • 3
    \$\begingroup\$ var.isdigit() might be faster than var in DIGITS, since it is a built-in and probably implemented in C. \$\endgroup\$ – Graipher Feb 18 '18 at 16:19
  • \$\begingroup\$ Good point indeed! \$\endgroup\$ – SylvainD Feb 18 '18 at 16:21
  • \$\begingroup\$ except that uses a different definition of integer digits than '0123456789' \$\endgroup\$ – Snowbody Feb 19 '18 at 6:13
  • \$\begingroup\$ Exceptions shouldn't go to the same stream as the program output. Look into using the logging module \$\endgroup\$ – Snowbody Feb 19 '18 at 14:16
  • \$\begingroup\$ For the test harness I'd use a list of tuples instead of a dict since we're not ever indexing by key. That also removes the need for .items() \$\endgroup\$ – Snowbody Feb 19 '18 at 14:18
5
\$\begingroup\$

your english is more than good ;)

Very small comments to give some advice at this level:

  • Try a bit better naming for functions, is vital for anyone that will read your code to help understand them. Imagine someone has to read for example your first function Numbers(var) and try and figure out what it does just with that name. I would suggest a naming like is_digit(character) where is easy to understand what the functions does (checks if something is digit) and what kind of parameter it expects (a single character)
  • Tedious comparisons of a single character, like var == '0' or... can be better written using python in operator. What about var in '0123456789'
  • Same naming you can use for def operator(operato):, what about is_operator(character)
  • The content of the while clause is a bit big, you may consider dividing it into functions

Last but not least

#created by Estoria Wandertag, while he was sitting on the toilet and thought about life and programming

That's a very literal approach to dumping data in the internet ;)

\$\endgroup\$
  • \$\begingroup\$ Thank you for advising me. I should definetely name the functions somehow else than that, you're totally right. I didnt know, that there was an in operator... :D To the fourth mentioned point: Are there negative effects on the while loop, or is it just clearer? I wanted to that and Thanks! (to the last point: It's True and there aren't that many people who would want to substitude my name with theirs ^^) \$\endgroup\$ – Estoria Wandertag Feb 18 '18 at 6:53
  • \$\begingroup\$ Oh and it's not my English, but it's the editors English (I forgot apostrophies and he wrote 'straightforward'. Then I was confused because I didn't write 'straightforward') \$\endgroup\$ – Estoria Wandertag Feb 18 '18 at 7:02
  • \$\begingroup\$ It's more clear for anyone reading the code, including you :) \$\endgroup\$ – A. Romeu Feb 18 '18 at 10:58
5
\$\begingroup\$

When you're writing a computer program in any language, one of your goals should be to eliminate duplicated code. Computers are good at doing the same task over and over again, even with slightly different data.

A. Romeu has pointed out that you can use Python's in operator to find if a character is one of a set of characters. But there are other places where Python includes language features that could greatly simplify your code.

For instance, take a look at your operation() function. Note that the code is repeated, with just the operator being different:

def operation(string, num1, num2):
    if string == '+':
        return num1 + num2
    if string == '-':
        return num1-num2
    if string == '*':
        return num1*num2
    if string == '/':
        return num1/num2
    if string == '^':
        return num1 ** num2

The usual way to do something like this in Python is to pre-construct a dict mapping the items to functions performing those actions, then pull the functions out of the dict.

In this case, since you're using operators, and those can't be stored in a dict, you'll need to import operator to get the function versions of the operators. Then below the import statement you can say

operations = {
    '+' : operator.add,
    '-' : operator.sub,
    '*' : operator.mul,
    '/' : operator.truediv}

def operation(operator, num1, num2):
    return operations[operator](num1, num2)

No more duplicated code!

As for your main code, there are a number of things to improve, but I want to focus on the duplicated code. You have lines saying:

    number1 = Test4Num(char)[0]
    # ...
    end_number1 = Test4Num(char)[1]

so you have two calls to Test4Num(char). Instead, use Python's multiple assignment feature (also called tuple assignment):

    (number1, end_number1) = Test4Num(char)

This assigns to both number and end_number1.

Also I think your final print() is unnecessary; it prints out partial intermediate results just run together; you just want the end result, right?

\$\endgroup\$
  • \$\begingroup\$ wow, thanks a lot! I made the code in an app, where I wasn't sure, if I was able to import things. I didn't know the second improvement proposal, thanks again. But the print() was meant, because I wanted to see all results in between and show, that it doesnt calculate '/' or '*' before '+' or '-'. Thanks! \$\endgroup\$ – Estoria Wandertag Feb 18 '18 at 6:45
  • \$\begingroup\$ Importing is part and parcel of Python. If your app doesn't let you use import, it's not giving you Python. \$\endgroup\$ – Snowbody Feb 19 '18 at 6:14
  • \$\begingroup\$ Debugging statements should be separate from the rest of your code, for instance using the logging module via import logging. \$\endgroup\$ – Snowbody Feb 19 '18 at 6:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.