开发者

OOP Design - In Python, is this a quality OO Design or an epic fail?

In a system that accepts orders which have payments which have gateway transactions should the objects be like this:

class Order(object):
        ... Inside init ...
        self.total_in_dollars = <Dollar Amount>
        self.is_paid = <Boolean Value>

class Payment(object):
        ... Inside init ...
        self.order = order_instance
        self.amount = order.total_in_dollars

class GatewayTransaction(object):
        ... Inside init ...
        self.payment = payment_instance
        self.amount = <Dollar Amount>

It seems this would be the way to do it (obviously this is not real code with integer dollar amounts, etc but you get the picture). I did it this way because an order can exist without payment and a payment can exist before an actual PayPal transaction occurs. Does this lack in your opinion? Am I backwards in my thinking?

OR, Should it be more like this:

class GatewayTransaction(object):
    payment = payment_instance
    amount = <Dollar Amount>

class Payment(object):
    amount = <Dollar Amount>
    gateway_transaction = gateway_transaction_instance

class Order(object):
    amount_in_dollars 开发者_StackOverflow= <Dollar Amount>
    payment = payment_instance


You appear to be assigning what should obviously be instance variables as class variables, which is clearly a very wrong tack to take. IOW, the variables should be self.total_in_dollars (for instance of Order) and so forth, assigned in __init__, not class-variables, assigned in the class statement!

Simply creating an instance of Order without a corresponding one of Payment is fine (and should obviously set is_paid to False), based just on the total (and presumably some numeric ID so customers &c can in the future refer to a specific order).

Don't duplicate information needlessly! Since a Payment instance will always have a reference to an Order instance, it should not copy self.order.total_in_dollars over to self.amount -- better to have that information in one place (you can make a readonly property if you want to access it nicely); and even more so for the transaction instances.

If an Order instance carried further metadata that influences how the corresponding Payment instance is created and behaves, that's OK, but strongly suggests making the creation of the Payment instance the job of a factory-method of class Object (which can then also keep track of already-generated instances and ensure there will never be more than one Payment instance for a single given Order instance).

Edit: now that the OP has somewhat edited the A, I can confirm the opinion that the dependencies in the first version are roughly correct (except, again, the amount should not get copied all over the place) and those in the second versions are, prima facie, not correct (the presence of a mutual/circular dependency for example is always a design smell unless clearly and explicitly justified by special application needs -- even when the need to navigate back and forth exists, one of the two links should be a weak reference, at least).

Edit: as the OP explicitly asks for more detail on the factory method I suggested, what I had in mind was something like this:

import weakref

class Payment(object):
  def __init__(self, order):
    self.order = weakref.proxy(order, self.ordergone)
  def ordergone(self, *_):
    self.order = None
  @property
  def amount(self):
    if self.order is None: return None
    else: return self.order.total_in_dollars

class Order(object):
  def __init__(self, amount):
    self.total_in_dollars = amount
    self.is_paid = False
    self._payment = None
  @property
  def payment(self):
    if self._payment is None:
      self._payment = Payment(self)
    return self._payment


Give the objects constructors that take the appropriate other object that you want it to refer to, then make the fields into properties that verify the type you assign to them.


I would approach this differently - by writing the some code for dealing with Orders, Payments etc. This would clarify my design needs, e.g. it would turn out that Payment.amount could be greater that Order.total_in_dollars, because of some processing fees. But then, it could turn out that maybe those processing fees should be stored separately, or even they could/should have their own model. And yes, this is TDD.

0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜