Skip to main content
Typo
Source Link
crazyGuy
  • 221
  • 1
  • 3

Review regarding the main question: reducing the ifs

Let me start by saying that Python does not support switch statements, which would have been very useful in this case. I would recommend reading this StackOverflow answer on how to get a similar result (I will be using that approach here too).

Something you should notice is that your if statements:

if command == "add":
    print(float(arg1) + float(arg2))

are running code that follows this pattern:

print(arg operand arg)

So it follows the question: how do we generalise that?

It turns out that we can use the operator to our advantage here:

import operator
def operation(a, b, operand):
   return operand(a, b)

Then you can map your "commands" to the different operands as such:

def get_operator(x):
   return {
     'add': operator.add,
     'multiply': operator.mul,
     ...
   }[x]

and finally you can reduce your multiple ifs with a single line of code:

operation(arg1, arg2, get_operator(command))

It must be noted that you could also have written:

get_operator(command)(arg1, arg2)

One can argue that one approach is better than the other for different reasons related to abstractions, decomposition, etc...

General Review

Argparse

Rather than doing all that tokenisation of the commands yourself, you should be using the argparse module. I am not going to explain here how to convert your code to use argparse, but if you follow the tutorial in the documentation it should be straighforward how to do that. That will also massively reduce the code you have written for the help menu.

Naming

In Python it is good convention to use snake_case for naming variables and functions. So the function SavetoDisk would become save_to_disk.

Lists and Enums

Your commandList is not a list. It is just a string separated by the | character. Rather consider using a true list and have enums rather than strings for your commands.

commands = [ Command.ADD, Command.MULTIPLY, ...]

Best of luck!


Update on Python Pattern Matching

I just found out that the pattern matching proposals (PEP 636 and companions PEP 634, PEP 635) and have been accepted for development just yesterday 08/02/2021!

Review regarding the main question: reducing the ifs

Let me start by saying that Python does not support switch statements, which would have been very useful in this case. I would recommend reading this StackOverflow answer on how to get a similar result (I will be using that approach here too).

Something you should notice is that your if statements:

if command == "add":
    print(float(arg1) + float(arg2))

are running code that follows this pattern:

print(arg operand arg)

So it follows the question: how do we generalise that?

It turns out that we can use the operator to our advantage here:

import operator
def operation(a, b, operand):
   return operand(a, b)

Then you can map your "commands" to the different operands as such:

def get_operator(x):
   return {
     'add': operator.add,
     'multiply': operator.mul,
     ...
   }[x]

and finally you can reduce your multiple ifs with a single line of code:

operation(arg1, arg2, get_operator(command))

It must be noted that you could also have written:

get_operator(command)(arg1, arg2)

One can argue that one approach is better than the other for different reasons related to abstractions, decomposition, etc...

General Review

Argparse

Rather than doing all that tokenisation of the commands yourself, you should be using the argparse module. I am not going to explain here how to convert your code to use argparse, but if you follow the tutorial in the documentation it should be straighforward how to do that. That will also massively reduce the code you have written for the help menu.

Naming

In Python it is good convention to use snake_case for naming variables and functions. So the function SavetoDisk would become save_to_disk.

Lists and Enums

Your commandList is not a list. It is just a string separated by the | character. Rather consider using a true list and have enums rather than strings for your commands.

commands = [ Command.ADD, Command.MULTIPLY, ...]

Best of luck!

Review regarding the main question: reducing the ifs

Let me start by saying that Python does not support switch statements, which would have been very useful in this case. I would recommend reading this StackOverflow answer on how to get a similar result (I will be using that approach here too).

Something you should notice is that your if statements:

if command == "add":
    print(float(arg1) + float(arg2))

are running code that follows this pattern:

print(arg operand arg)

So it follows the question: how do we generalise that?

It turns out that we can use the operator to our advantage here:

import operator
def operation(a, b, operand):
   return operand(a, b)

Then you can map your "commands" to the different operands as such:

def get_operator(x):
   return {
     'add': operator.add,
     'multiply': operator.mul,
     ...
   }[x]

and finally you can reduce your multiple ifs with a single line of code:

operation(arg1, arg2, get_operator(command))

It must be noted that you could also have written:

get_operator(command)(arg1, arg2)

One can argue that one approach is better than the other for different reasons related to abstractions, decomposition, etc...

General Review

Argparse

Rather than doing all that tokenisation of the commands yourself, you should be using the argparse module. I am not going to explain here how to convert your code to use argparse, but if you follow the tutorial in the documentation it should be straighforward how to do that. That will also massively reduce the code you have written for the help menu.

Naming

In Python it is good convention to use snake_case for naming variables and functions. So the function SavetoDisk would become save_to_disk.

Lists and Enums

Your commandList is not a list. It is just a string separated by the | character. Rather consider using a true list and have enums rather than strings for your commands.

commands = [ Command.ADD, Command.MULTIPLY, ...]

Best of luck!


Update on Python Pattern Matching

I just found out that the pattern matching proposals (PEP 636 and companions PEP 634, PEP 635) and have been accepted for development just yesterday 08/02/2021!

Source Link
crazyGuy
  • 221
  • 1
  • 3

Review regarding the main question: reducing the ifs

Let me start by saying that Python does not support switch statements, which would have been very useful in this case. I would recommend reading this StackOverflow answer on how to get a similar result (I will be using that approach here too).

Something you should notice is that your if statements:

if command == "add":
    print(float(arg1) + float(arg2))

are running code that follows this pattern:

print(arg operand arg)

So it follows the question: how do we generalise that?

It turns out that we can use the operator to our advantage here:

import operator
def operation(a, b, operand):
   return operand(a, b)

Then you can map your "commands" to the different operands as such:

def get_operator(x):
   return {
     'add': operator.add,
     'multiply': operator.mul,
     ...
   }[x]

and finally you can reduce your multiple ifs with a single line of code:

operation(arg1, arg2, get_operator(command))

It must be noted that you could also have written:

get_operator(command)(arg1, arg2)

One can argue that one approach is better than the other for different reasons related to abstractions, decomposition, etc...

General Review

Argparse

Rather than doing all that tokenisation of the commands yourself, you should be using the argparse module. I am not going to explain here how to convert your code to use argparse, but if you follow the tutorial in the documentation it should be straighforward how to do that. That will also massively reduce the code you have written for the help menu.

Naming

In Python it is good convention to use snake_case for naming variables and functions. So the function SavetoDisk would become save_to_disk.

Lists and Enums

Your commandList is not a list. It is just a string separated by the | character. Rather consider using a true list and have enums rather than strings for your commands.

commands = [ Command.ADD, Command.MULTIPLY, ...]

Best of luck!