So I was checking out the courses on InteractivePython, and I came across this example to demonstrate inheritance.
My focus is on the Connector
class and how it interacts with BinaryGate
and UnaryGate
via the setNextPin
method.
class LogicGate:
def __init__(self,n):
self.name = n
self.output = None
def getName(self):
return self.name
def getOutput(self):
self.output = self.performGateLogic()
return self.output
class BinaryGate(LogicGate):
def __init__(self,n):
LogicGate.__init__(self,n)
self.pinA = None
self.pinB = None
def getPinA(self):
if self.pinA == None:
return int(input("Enter Pin A input for gate "+self.getName()+"-->"))
else:
return self.pinA.getFrom().getOutput()
def getPinB(self):
if self.pinB == None:
return int(input("Enter Pin B input for gate "+self.getName()+"-->"))
else:
return self.pinB.getFrom().getOutput()
def setNextPin(self,source):
if self.pinA == None:
self.pinA = source
else:
if self.pinB == None:
self.pinB = source
else:
print("Cannot Connect: NO EMPTY PINS on this gate")
class AndGate(BinaryGate):
def __init__(self,n):
BinaryGate.__init__(self,n)
def performGateLogic(self):
a = self.getPinA()
b = self.getPinB()
if a==1 and b==1:
return 1
else:
return 0
class OrGate(BinaryGate):
def __init__(self,n):
BinaryGate.__init__(self,n)
def performGateLogic(self):
a = self.getPinA()
b = self.getPinB()
if a ==1 or b==1:
return 1
else:
return 0
class UnaryGate(LogicGate):
def __init__(self,n):
LogicGate.__init__(self,n)
self.pin = None
def getPin(self):
if self.pin == None:
return int(input("Enter Pin input for gate "+self.getName()+"-->"))
else:
return self.pin.getFrom().getOutput()
def setNextPin(self,source):
if self.pin == None:
self.pin = source
else:
print("Cannot Connect: NO EMPTY PINS on this gate")
class NotGate(UnaryGate):
def __init__(self,n):
UnaryGate.__init__(self,n)
def performGateLogic(self):
if self.getPin():
return 0
else:
return 1
class Connector:
def __init__(self, fgate, tgate):
self.fromgate = fgate
self.togate = tgate
tgate.setNextPin(self)
def getFrom(self):
return self.fromgate
def getTo(self):
return self.togate
def main():
g1 = AndGate("G1")
g2 = AndGate("G2")
g3 = OrGate("G3")
g4 = NotGate("G4")
c1 = Connector(g1,g3)
c2 = Connector(g2,g3)
c3 = Connector(g3,g4)
print(g4.getOutput())
main()
And so the output might look like this:
Input Pin A for gate G1 --> 0
Input Pin B for gate G1 --> 0
Input Pin A for gate G2 --> 1
Input Pin B for gate G2 --> 0
1
So everything is working as intended, but my concern is the "two-way access" between, for example, BinaryGate
and Connector
.
It's bothering me that BinaryGate.getPinA
can call (Connector).getFrom().getOutput()
in the line return self.pinA.getFrom().getOutput()
, while Connector.__init__()
calls BinaryGate.setNextPin
In my mind, there should only be one direction of "visibility" between the two objects - either a *Gate
can access members of a Connector
, or vice-versa, but not both ways at once. Is this a flawed assumption?
Does the code as-is break encapsulation? If so, how can the code be refactored to have proper encapsulation? If not, why not?
Edit:
Just to be clear for future viewers, this is not my code, it's from InteractivePython's Data Structures course.
3 Answers 3
I think you're too worried about sticking to traditional OOP principles in Python. For instance, your getName
function is totally unnecessary. Python doesn't have private variables, so any of your attributes can just be accessed by gate.name
without needing a dedicated function. Likewise, getOutput
seems redundant when you can just call gate.performGateLogic()
directly and get the same result.
Python doesn't really enforce strict encapsulation, it leaves everything open for you to access as you'd like. That's why you have the option to have that two way access you're concerned about. Essentially, don't worry about it. Python is designed this way. If you think it's a significant concern (I personally don't), you could rearrange your code so that you're not using it bidirectionally. But even if you remove your use the possibility will still exist.
Also LogicGate
doesn't have performGateLogic
yet it calls it. To make this airtight, you should define a basic performGateLogic
function in LogicGate
that simply raises NotImplementedError
. This error indicates that a method should be overridden in inherited classes.
Also don't use n
as a parameter name in your __init__
s, just use name
as it's far more readable.
class LogicGate:
def __init__(self, name):
self.name = name
self.output = None
def performGateLogic(self):
raise NotImplementedError
Instead of using == None
it's more Pythonic and readable to test is None
.
And in setNextPin
you could use an elif
instead of nesting an if
inside an else block:
def setNextPin(self,source):
if self.pinA is None:
self.pinA = source
elif self.pinB is None:
self.pinB = source
else:
print("Cannot Connect: NO EMPTY PINS on this gate")
If the first block is not True
, the elif
block is tested. If that's not True
either then the else
is executed.
Why are you setting pins a and b to 0 and 1? You could just use True
and False
to be more readable and it seems like that is a more accurate representation of what the values mean. In particular, the gate logic in AndGate
would be easier to perform this way:
def performGateLogic(self):
a = self.getPinA()
b = self.getPinB()
return a and b
If a
and b
are booleans, you can just directly return the result of them to get either True
or False
. You could even make this a one liner if you're so inclined:
def performGateLogic(self):
return self.getPinA() and self.getPinB()
-
\$\begingroup\$ Thanks for the feedback - do you have any comments about my questions I raised at the bottom of my post? \$\endgroup\$Kyle Pittman– Kyle Pittman2015年12月01日 16:53:37 +00:00Commented Dec 1, 2015 at 16:53
-
\$\begingroup\$ @Monkpit I tried to answer it in my first paragraph but I think it was mixed up with another point so I gave a more explicit answer. \$\endgroup\$SuperBiasedMan– SuperBiasedMan2015年12月01日 17:04:17 +00:00Commented Dec 1, 2015 at 17:04
-
\$\begingroup\$ Thanks for clarifying, I've marked your answer correct as it addresses my code as well as the concerns I raised. Cheers! \$\endgroup\$Kyle Pittman– Kyle Pittman2015年12月01日 18:30:20 +00:00Commented Dec 1, 2015 at 18:30
You are mixing up lots of different responsibilities in your classes. For instance, AndGate
has to handle both the And
part and all the pin input parts. It is weird that an AndGate
doesn't actually take references to its input pins. Furthermore, how in this model do we connect gates to each other? By your design, a "pin" is a value comparable to an integer. What if I wanted to model something like (picking a random circuit from google):
How would I do that in your system? The Connector
s confuse the logic, since in your example you're creating a bunch of gates that are floating around - and then just to see that G1
and G2
go into G3
is split across several lines, rather than ideally:
g3 = OrGate('G3', g1, g2)
As a first go, gates should know their inputs and be able to perform logic to determine their outputs:
class NandGate(LogicGate):
def __init__(self, name, a, b):
LogicGate.__init__(self, name)
self.a = a
self.b = b
@property
def output(self):
return not (self.a.output and self.b.output)
This separates the concern of input handling (determining the pins) from the concern of actually emulating the gates (determining the outputs) - and is what will let us have NandGate
s as input to other NandGate
s:
A = ConstantPin('A', True)
B = ConstantPin('B', False)
N1 = NandGate('N1', A, B)
N2 = NandGate('N2', A, N1)
N3 = NandGate('N3', N1, B)
N4 = NandGate('N4', N2, N3)
N5 = NandGate('N5', N4, N4)
Q = N5.output
I have some comments regarding the base structure of your classes:
- I would rather connect the different gates directly to each other, rather than using a connector class which does a somewhat vague connection
- I would not have the class generate it's own input, like in
getPin()
methods, but rather use the already linked gate(s), down to aConstantGate
where I would set it either programmatically or based upon some input mechanism - You could build a recursive
__repr__
which would present the entire expression. I have not done in the code below, but this would be a useful extension if you're using this code to build logical expressions It makes sense to allow the constructor to set the inputs/pins directly, as that allows for building expressions directly, like if you wanted the expression
a and (b or c)
it could be written asAndGate(a, OrGate(b, c))
, wherea, b, and c
whereConstantGate
objects and easily changeable.The example of Barry could be written as:
a = ConstantGate(True) b = ConstantGate(False) N1 = NandGate(a, b) N4 = NandGate(NandGate(a, N1), NandGate(N1, b)) Q = NandGate(N4, N4)
With a proper defintion of
NandGate
class, you could now change the values ofa
andb
and see directly the effect onQ
.You could/should provide validation that with the exception for
ConstantGate
all the other gates should be variations of theLogicGate
class, and possibly provide better redefinition mechanism
But here is my rudimentary suggestion for building the tree of logical gates:
class LogicGate(object):
"""The base class for building logical expressions."""
_gate_counter = 0
def __init__(self, name, prefix='L'):
LogicGate._gate_counter += 1
# Set default name of gate to gate_prefix and global gate_counter
if name is not None:
self._name = name
else:
self._name = '{}{}'.format(prefix, LogicGate._gate_counter)
def output():
"""Should return the logical result for the gate."""
raise NotImplementedError
def redefine(self):
"""Redefines the optional input gates for the gate."""
raise NotImplementedError
def get_name(self):
"""Return name of gate, if set."""
return self._name
def __repr__(self):
return '{}:{} '.format(self._name, self.output())
def __str__(self):
return str(self.output())
class UnaryGate(LogicGate):
"""A LogicGate have only one input."""
def __init__(self, gate, name=None, prefix='U'):
super(UnaryGate, self).__init__(name, prefix)
self._gate = gate
def output(self):
return self._gate.output()
def redefine(self, gate):
self._gate = gate
class ConstantGate(UnaryGate):
"""A UnaryGate having a set value, the starter of the logic chains."""
def __init__(self, bool_value, name=None, prefix='C'):
super(ConstantGate, self).__init__(None, name, prefix)
self._value = bool_value
def output(self):
return self._value
def redefine(self, bool_value):
self._value = bool_value
class NotGate(UnaryGate):
"""A UnaryGate negating the given input gate."""
def __init__(self, logic_gate, name=None, prefix='^'):
super(UnaryGate, self).__init__(name, prefix)
self._gate = logic_gate
def output(self):
return not self._gate.output()
class BinaryGate(LogicGate):
"""A LogicGate having two inputs."""
def __init__(self, first_logic_gate, second_logic_gate, name=None, prefix='B'):
super(BinaryGate, self).__init__(name, prefix)
self._first_gate = first_logic_gate
self._second_gate = second_logic_gate
def output(self):
raise NotImplementedError
def redefine(self, first_logic_gate=None, second_logic_gate=None):
if first_logic_gate is not None:
self._first_gate = first_logic_gate
if second_logic_gate is not None:
self._second_gate = second_logic_gate
class AndGate(BinaryGate):
"""A BinaryGate and'ing the gates together."""
def __init__(self, first_logic_gate, second_logic_gate, name=None, prefix='A'):
super(AndGate, self).__init__(first_logic_gate, second_logic_gate,
name, prefix)
def output(self):
return self._first_gate.output() and self._second_gate.output()
class OrGate(BinaryGate):
"""A BinaryGate or'ing the gates together."""
def __init__(self, first_logic_gate, second_logic_gate, name=None, prefix='O'):
super(OrGate, self).__init__(first_logic_gate, second_logic_gate,
name, prefix)
def output(self):
return self._first_gate.output() or self._second_gate.output()
class NandGate(BinaryGate):
"""A BinaryGate doing a NAND, that is reverse the and'ing of gates."""
def __init__(self, first_logic_gate, second_logic_gate, name=None, prefix='O'):
super(NandGate, self).__init__(first_logic_gate, second_logic_gate,
name, prefix)
def output(self):
return not(self._first_gate.output() and self._second_gate.output())
FORMAT = u'{:<8}{:<8}{:<8}{:<8}{:<8}{:<8}{:<8}{:<8}'
def main():
# Build some more or less complex logical expressions
the_a = ConstantGate(False)
the_b = ConstantGate(False)
the_not = NotGate(the_a)
the_and = AndGate(the_a, the_b)
the_or = OrGate(the_a, the_b)
the_complex = AndGate(the_not, the_or)
the_nand_1 = NotGate(the_and)
the_nand_2 = NandGate(the_a, the_b)
# Print the truth table for these expressions
print(FORMAT.format('a', 'b', '^a', 'ab', 'a+b', '^a(a+b)', '^(ab)', u'a\u22bcb'))
for a, b in ((False, False), (False, True), (True, False), (True, True)):
the_a.redefine(a)
the_b.redefine(b)
print(FORMAT.format(the_a, the_b, the_not, the_and, the_or,
the_complex, the_nand_1, the_nand_2))
if __name__ == '__main__':
main()
If you run this file, as is, you'll get the following truth table:
a b ^a ab a+b ^a(a+b) ^(ab) a⊼b
False False True False False False True True
False True True False True True True True
True False False False True False True True
True True False True True False False False
Hope this gives you some idea on another apporach related to how to do object orientation in Python.