0

I think this will be most easily demonstrated with an example, but the question is a general one. Say I am using a library like PyVISA, which interfaces GPIB devices with my program. I have set up a python class for each instrument, so for a power supply, I might have something like this:

import visa

class PowerSupply:
    def __init__(self):
        rm = visa.ResourceManager()
        self.ps = rm.open_resource('GPIB0::12::INSTR')
    def getVoltage(self):
        return self.ps.ask('VOLT?')
    def setVoltage(self,v):
        self.ps.write('VOLT '+str(v))
    ...

ps = PowerSupply()
ps.setVoltage(10)

Unfortunately, there is the chance that the rm.open_resource function may not work, or might return None if a device does not exist at that address (in my code I actually wrote a function that did this instead). My question is: what is the best practice for coding a class like PowerSupply? One could write exceptions into every method that test if self.ps exists/is not None, but it seems there must be a better way. Is there?!

roboguy222
  • 157
  • 13
  • 1
    Surely `__init__` should fail in that case, throwing an error? – jonrsharpe Oct 08 '14 at 18:27
  • to further jon's comment ... it should be the caller's job to manage the dependency `try:ps=PowerSupply(); except: do_something_else()` ... and init should certainly raise an error about no powersupply found – Joran Beasley Oct 08 '14 at 18:30
  • 1
    @jonrsharpe: The question says that `open_resource` "might return `None`". So no, `__init__` wouldn't fail in that case. But the fix is to _make_ it fail in that case. Add a `if not self.ps: raise SomeException('failed to open resource')` or the like. – abarnert Oct 08 '14 at 19:36
  • You could decouple `PowerSupply` from your resource even more by injecting it into the `__init__()` method instead of constructing it in there. This way you can switch the implementation as long as the interface is compatible. – P3trus Jan 13 '15 at 16:43

1 Answers1

2

One could write exceptions into every method that test if self.ps exists/is not None, but it seems there must be a better way. Is there?!

Yes. The way you've written your code, if self.ps is ever None, it's going to be None from the start, and never change again. So, instead of testing it in every method, just test it once:

def __init__(self):
    rm = visa.ResourceManager()
    self.ps = rm.open_resource('GPIB0::12::INSTR')
    if self.ps is None:
        raise PowerSupplyException('Unable to open resource GPIB0::12::INSTR')

Now, any code that constructs a PowerSupply has to either handle that exception or be prepared to propagate it, but your question implies that open_resource might raise an exception anyway, so that can't be a problem. Besides, that seems like about the right place to handle it—at the top level of your program, wherever you're creating a PowerSupply in response to some GUI or CLI or network command, that's where you're prepared to handle being unable to create it, right?

And any code that uses an already-constructed PowerSupply doesn't have anything to worry about. You know self.ps is not None or you couldn't have gotten the object back from the constructor.

abarnert
  • 354,177
  • 51
  • 601
  • 671