3
\$\begingroup\$

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.

asked Dec 1, 2015 at 14:53
\$\endgroup\$

3 Answers 3

1
\$\begingroup\$

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()
holroy
11.7k1 gold badge27 silver badges59 bronze badges
answered Dec 1, 2015 at 16:43
\$\endgroup\$
3
  • \$\begingroup\$ Thanks for the feedback - do you have any comments about my questions I raised at the bottom of my post? \$\endgroup\$ Commented 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\$ Commented 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\$ Commented Dec 1, 2015 at 18:30
3
\$\begingroup\$

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):

enter image description here

How would I do that in your system? The Connectors 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 NandGates as input to other NandGates:

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
answered Dec 1, 2015 at 17:28
\$\endgroup\$
1
\$\begingroup\$

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 a ConstantGate 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 as AndGate(a, OrGate(b, c)), where a, b, and c where ConstantGate 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 of a and b and see directly the effect on Q.

  • You could/should provide validation that with the exception for ConstantGate all the other gates should be variations of the LogicGate 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.

answered Dec 1, 2015 at 23:59
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.