Electricity Calculator

Question

This is a calculator for Ohm's Law and Power Law. It works but I'd like some suggestions on making the code look a little cleaner, or run more efficiently.

 # ohm's law calculator

#These functions perform ohm's law and power law calculations based on variables from user input.
def voltage(i, r):
    return i * r



def current(r, v):
    return r / v


def resistance(i, v):
    return i / v



def power(v, i):
    return v * i  



def pvolt(w, i):
    return w / i



def pcurrent(w, v):
    return w / v



# First user interface menu. let's user choose between ohm's law and power law calculations.   

print "Ohm's Law Calculations:"

print "1. Voltage"
print "2. Current"
print "3. Resistance"

print "Power Law Calculations:"

print "4. Wattage"
print "5. Voltage"
print "6. Current"

#Line 48 is taking the user's menu choice and passing it to the "choice" options below.
choice = raw_input(">>> ")



#These if statements are calling the functions.

#This gives values to "i" "v"  "w" and "r" from user input as floating point numbers the print line calls the "voltage" function and prints out the results of its calculation using the "format" function which automatically displays the needed decimal places.



if choice == "1":
    i=float(raw_input("Enter current:"))
    r=float(raw_input("Enter resistance:"))
    print "{0} volts" .format(voltage(i, r))


elif choice == "2":
    r=float(raw_input("Enter resistance:"))
    v=float(raw_input("Enter voltage:"))
    print "{0} amps" .format(current(r, v))



elif choice == "3":  
    i=float(raw_input("Enter current:"))
    v=float(raw_input("Enter voltage:"))
    print "{0} ohms" .format(resistance(i, v))



elif choice == "4":
    i=float(raw_input("Enter current:"))
    v=float(raw_input("Enter voltage:"))
    print "{0} watts" .format(power(i, v))


#Line 82 solves voltage according to power law
elif choice == "5":
    w=float(raw_input("Enter wattage:"))
    i=float(raw_input("Enter current:"))
    print "{0} volts" .format(pvolt(w, i))


#Line 89 solves current according to power law. I am attempting to make it round of the answer
elif choice == "6":
    w=float(raw_input("Enter wattage:"))
    v=float(raw_input("Enter voltage:"))
    print "{0} amps" .format(pcurrent(w, v))  





#This line is here partly because its funny, and partly because I thought it would be cool to create my own error message   

else:
    print  "Invalid number. Your computer will now self destruct."

You could remove some of those empty lines, so the code would be half as many lines and much easier to inspect, when more of it can be visible on the screen at the same time... — hyde, Sep 18, 2014 at 7:37

jsanc623 · Accepted Answer · 2014-09-17 20:08:08Z

You should think more about how you arrange and style your code. Additionally, you should be aware of DRY methodology and keep from repeating code wherever possible.

I made a couple of modifications to your code. Primarily, I cleaned up the arrangement and styling. After that, I added an Ohm class - which places all of your Ohm related functions as methods (again for code cleanliness):

"""
This class contains functions which perform Ohm's law and power 
law calculations based on variables from user input
"""
class Ohm():
    @staticmethod
    def voltage(i, r): return i * r

    @staticmethod
    def current(r, v): return r / v

    @staticmethod
    def resistance(i, v): return i / v

    @staticmethod
    def power(v, i): return v * i  

    @staticmethod
    def pvolt(w, i): return w / i

    @staticmethod
    def pcurrent(w, v): return w / v

After that, I changed how you output the block of text from a bunch of print statements, to 1 multiline print statement:

# First user interface menu. let's user choose between ohm's law and power law calculations.   
print """Ohm's Law Calculations:
    1. Voltage
    2. Current
    3. Resistance

Power Law Calculations:
    4. Wattage
    5. Voltage
    6. Current
"""

After that, I added a call to int() on the choice = raw_input(...) call so that all input is shunted to an integer. After that, we initialize our Ohm class to an Ohm variable. Then, instead of having multiple definitions for i, r, v, and w - we ask if the choice is in a list, define i/r/v/w. This lessens the amount of duplicated code:

### Since we're asking for the current in multiple locations, simplify it
if choice in [1, 3, 4, 5]:
    i = float(raw_input("Enter current:"))

### Again, since we're asking for voltage in multiple locations, simplify it
if choice in [2, 3, 4, 6]:
    v = float(raw_input("Enter voltage:"))

### Simplifying input for resistance
if choice in [1, 2]:
    r = float(raw_input("Enter resistance:"))

## And lastly, simplifying for wattage
if choice in [5, 6]:
    w = float(raw_input("Enter wattage:"))

After that, we continue with an if/else block which calls the function according to the user input. Our i/v/r/w should already be defined in the block above. One major change here is that we are no longer printing directly from here. Rather we are setting the output in an output variable, which we later use to print.

output = None
if choice == 1:
    output = "{0} volts".format(Ohm.voltage(i, r))
elif choice == 2:
    output = "{0} amps".format(Ohm.current(r, v))
elif choice == 3:  
    output = "{0} ohms".format(Ohm.resistance(i, v))
elif choice == 4:
    output = "{0} watts".format(Ohm.power(i, v))
elif choice == 5:
    output = "{0} volts".format(Ohm.pvolt(w, i))
elif choice == 6:
    output = "{0} amps".format(Ohm.pcurrent(w, v))

Finally, we check if output variable is None. If it is not None, then we print it, otherwise we print an error. As for your 'funny' error, remember that error messages should be informative (ie, should tell the end user where the error occurred and/or why it occurred):

if output is not None:
    print output
else:
    print "Invalid input values, please try again"

Lastly, here is all of the code put together (and a REPL):

"""
Ohm's law calculator
"""

"""
This class contains functions which perform Ohm's law and power 
law calculations based on variables from user input
"""
class Ohm():
    @staticmethod
    def voltage(i, r): return i * r

    @staticmethod
    def current(r, v): return r / v

    @staticmethod
    def resistance(i, v): return i / v

    @staticmethod
    def power(v, i): return v * i  

    @staticmethod
    def pvolt(w, i): return w / i

    @staticmethod
    def pcurrent(w, v): return w / v

# First user interface menu. let's user choose between ohm's law and power law calculations.   
print """Ohm's Law Calculations:
    1. Voltage
    2. Current
    3. Resistance

Power Law Calculations:
    4. Wattage
    5. Voltage
    6. Current
"""

# Grab the user input
choice = int(raw_input(">>> "))

# Initialize our Ohm class
Ohm = Ohm()

### Since we're asking for the current in multiple locations, simplify it
if choice in [1, 3, 4, 5]:
    i = float(raw_input("Enter current:"))

### Again, since we're asking for voltage in multiple locations, simplify it
if choice in [2, 3, 4, 6]:
    v = float(raw_input("Enter voltage:"))

### Simplifying input for resistance
if choice in [1, 2]:
    r = float(raw_input("Enter resistance:"))

## And lastly, simplifying for wattage
if choice in [5, 6]:
    w = float(raw_input("Enter wattage:"))

output = None
if choice == 1:
    output = "{0} volts".format(Ohm.voltage(i, r))
elif choice == 2:
    output = "{0} amps".format(Ohm.current(r, v))
elif choice == 3:  
    output = "{0} ohms".format(Ohm.resistance(i, v))
elif choice == 4:
    output = "{0} watts".format(Ohm.power(i, v))
elif choice == 5:
    output = "{0} volts".format(Ohm.pvolt(w, i))
elif choice == 6:
    output = "{0} amps".format(Ohm.pcurrent(w, v))  

if output is not None:
    print output
else:
    print "Invalid input values, please try again"

I think there is absolutely no reason to use a class at that point. It is a class which has only static methods. Use a module for that. — Jonas Schäfer, Sep 17, 2014 at 20:48
@JonasWielicki valid point. I was thinking more along the lines of Techandmusic extending the class in the future, but a module would work better as well. — jsanc623, Sep 17, 2014 at 20:54
@jsanc623 Along the lines of eliminating repitition, is there a way I could write one function, give it all three Ohm's law variables and have it solve for any of them, or is that something a little beyond my experience level? — user53237, Sep 18, 2014 at 18:21
You could do that - but remember that while eliminating repetition is good, having a function do more than 1 thing is bad (see SRP: en.wikipedia.org/wiki/Single_responsibility_principle) as it leads to more complicated code and additional possible points of failure. — jsanc623, Sep 18, 2014 at 18:39

mariosangiorgio · Accepted Answer · 2014-09-18 12:10:44Z

Why don't you use more self-explanatory names? For instance I'd rename voltage(i, r) to voltage(intensity, resistance). I know the symbols you used are quite standard but I think it is a good habit to give meaningful names to the variables you use.

I'd also suggest you to exploit the regular structure of your code to shrink it a bit. You can do that by creating a function that asks the user for a couple of values and executes a function.

def readInputAndExecuteOperation(firstVariableName, secondVariableName, operation, resultUnits):
    # here you should do some input checking/sanitization
    firstVariable=float(raw_input("Enter {0}:".format(firstVariableName)))
    secondVariable=float(raw_input("Enter {0}:".format(secondVariableName)))
    print "{0} {1}" .format(operation(firstVariable, secondVariable), resultUnits)

Once you have it, you're ready to compress all the duplicated code as follows. You create a list of tuples. Each tuple is in the form (functionName, firstVariableName, secondVariableName, functionToCall, resultUnit). Yes, you can pass around functions.

operations = [("voltage", "current", "resistance", voltage, "V"), ... ]

You can generate your menu by iterating over the list

def generateMenu(operations):
    for i in range(len(operations)):
        print "{0} {1}".format(i, operations[i][0])

You can read the choice of the user as you did, but you should change how you handle it to use the function we defined earlier.

choice = raw_input(">>> ")
#check that the choice is a valid number
operation = operations[choice]
readInputAndExecuteOperation(operation[1], operation[2], operation[3], operation[4])

functools.partial comes into mind for the operations list. Also, you’re not passing the 4th argument to readInputAndExecuteOperation. In addition, python prefers underscores over camelCase (c.f. pep 8) — Jonas Schäfer, Sep 17, 2014 at 20:49
It would be nice to use it. In fact I was thinking at is as a partial function but my knowledge of python is limited and I was too lazy to search to functools.partial. Thanks for your comment — mariosangiorgio, Sep 17, 2014 at 20:52

commanderdata · Accepted Answer · 2014-09-17 21:08:47Z

import sys

def get_current():
    return float(raw_input("Enter current:"))

def get_resistance():
    return float(raw_input("Enter resistance:"))

def get_voltage():
    return float(raw_input("Enter voltage:"))

def get_wattage():
    return float(raw_input("Enter wattage:"))

def voltage():
    print "{0} volts" .format(get_current()*get_resistance())

def current():
    print "{0} amps" .format(get_resistance()/get_voltage())

def resistance():
    print "{0} ohms" .format(get_current()/get_voltage())

def wattage():
    print "{0} watts" .format(get_current()*get_voltage())

def pvoltage():
    print "{0} volts" .format(get_wattage()/get_current())

def pcurrent():
    print "{0} amps" .format(get_wattage()/get_voltage())


options = { "1" : voltage,
            "2" : current,
            "3" : resistance,
            "4" : wattage,
            "5" : pvoltage,
            "6" : pcurrent,
} 

print "Ohm's Law Calculations:"

print "1. Voltage"
print "2. Current"
print "3. Resistance"

print "Power Law Calculations:"

print "4. Wattage"
print "5. Voltage"
print "6. Current"

choice = raw_input(">>> ")

try:
    options[choice]()
except KeyError:
    print  "Invalid number. Your computer will now self destruct."
    sys.exit()

I've replaced the if-else block with the python equivalent of a switch-case statement (which uses a dictionary for mapping a key and its value). Using a dictionary is generally more efficient and more concise than an if-else block.
Also notice that I've made a get functions, which improves the reusability of your code.
Finally, I've introduced exception handling, which is generally a more robust way of handling invalid input from a user.

“Using a dictionary is generally more efficient and more concise than an if-else block.” Concise yes, more efficient in many cases not (hashes need to be calculated, the dict needs to be built on every use if you don’t cache it manually). [i made a quick timing test with 5 string keys, and values sourced from the global scope; ifelse was about 2 times faster; but python is generally not the language for performance arguments]. Also, why did you rename pvoltage etc. to voltage2? This removes all information about what the difference between voltage and voltage2 is. — Jonas Schäfer, Sep 17, 2014 at 20:58
No good reason for changing pvoltage and pcurrent. I just corrected that. I suppose you're right about performance in this case, but if we were dealing with a much larger if-else block I would expect a dictionary to be more efficient. — commanderdata, Sep 17, 2014 at 21:11
Let the practical usefulness aside, this heavily depends on the input value distribution. If one (or a few) values are very likely, an if-else block can be much more efficient. Keep in mind, all the strings have to be processed at least once for hashing, and have to be inserted into the dictionary, which might even need to grow if the amount of values is large. However, this discussion isn’t of much value, as python is generally not a performance-centric language (read: it is slow) and it’s only about readability (and writability!) here. A dict clearly wins for any amount of values > 2. — Jonas Schäfer, Sep 17, 2014 at 21:15

Stack Exchange Network

Electricity Calculator

3 Answers 3

Your Answer

Hot Network Questions

Electricity Calculator

3 Answers 3

Your Answer

Sign up or log in

Post as a guest

Related

Hot Network Questions