Dealing with unwanted usages of a function

Question

I am writing a function that I would not like to get called given a certain context and am wondering how best to convey that to possible users of the function. Assume, for exemplification, I am writing a function process_payment that must not get called in an environment that demands certain security standards on the payments being done. Some possible options of dealing with this I have thought of:

1. Having a ...warning docstring comment ( or just a verbose comment in another programming language)

def process_payment(Payment payment):
"""
.. warning: DO NOT USE if we are in SECURE PAYMENT environment. Use a pipeline that leads to 
process_payment_securely() insead
"""
   [...]

2. Specifically checking if the conditions are met inside the process_payment function by calling in some global state to check the conditions.

def process_payment(Payment payment):
    if get_payment_state() == SECURE_PAYMENT:
        raise Exception(...)

3. Having the execution conditions be some values inside of a Payment class/struct, such that one can easily check them at the call site.

def process_payment(Payment payment):
    if payment.is_secure:
        raise Exception(...)

Obvious drawbacks to 1) are that it allows a not-careful user to process payments insecurely. Obvious drawbacks to 2) are that if the state which we are deciding with is quite far away in the call hierarchy, one needs to either call in some global state or propagate (potentially) a lot of information and pass it as an argument. Option 3) looks good, but one could imagine it being quite cumbersome and (depending on the programming language) suboptimal to include such redundant information into every Payment class.

Are there any other approaches I am missing? What are some common ways to deal with this?

Answer 1

Your requirement is that the function is called only within a context. But in none of the example you pass a context or a proxy for the context. The consequences are:

1. You outsource the context to the programmer and at implementation time. This is error prone: the more such insider knowledge is required, the higher the probability of errors, and the less reliable your system will become over time. In addition this doesn't work if the context is allowed to evolve dynamically at runtime.
2. You have only one context, the one that is held by the global state of your system. This proves to be very inflexible at run-time (e.g. What it you have different payment security requirements in same time? Or if the security requirement is shared with other processes than payments?), with the risk of side-effects that affect the reliability. In addition this creates a strong but hidden coupling between parts of the system that should be loosely coupled.
3. You outsource the context to the payment creation process, letting it duplicate some context information for your own convenience. This is could be very relevant, especially if the security level can really vary independently for every payment. However, this is not fully in line with the idea of separation of concerns, or single responsibility (in the sense "reason for change").

From your three options, #3 is the less harmful at least in the short run.

However, I'd first look for other suitable approaches, for example:

• Option 2 with an additional context argument (no globals) e.g. process_payment(payment, bank_security_context)
• Embedding payment_process in a class with the security context
• Embedding payment_process in a dedicated PaymentProvider class that implements a state pattern. The security level of the context is then provided to the PaymentProvider to change the state. The payment_process method would be overriden at the level of the concrete state class. The payment_process implementation of the states that are not compliant with expectations, would then raise the error.

Answer 2

Are there any other approaches I am missing? What are some common ways to deal with this?

You ignored the simplest and most common, which is to call the function process_payment_insecure or something similar. Naming conventions are often sufficient if they're well-designed and consistently applied.

In general, the approach of using a context argument (strongly-typed if possible) is superior, because it doesn't rely on the naming convention being policed manually at code/review time.

Answer 3

I would prefer to go with option 3., since this allows the client to check the state before calling the process_payment() function at all, and need to deal with an exception.

The implementation of payment.is_secure should be trivial:

def is_secure():
    return get_payment_state() == SECURE_PAYMENT:

But,- as you mentioned -, depending on the programming language that could get a bit cumbersome if you have independent payment class implementations (you have to implement is_secure() for each payment class type).

Fortunately almost all modern programming languages allow kinds of class inheritance and polymorphism. Thus it should be possible letting all payment class types having a common abstract base class, and implement the is_secure() function there once and only once.

Answer 4

I think you should take a step back from the architecture for a moment, and realize the payment processor should be secure or not. It sounds like you want to process payments in a secure environment, and not process them if the environment is not secure. Basically, if this is production or a beta testing environment used by real end users, process the payment. Don't process the payment otherwise.

Something further up the architecture, closer to the application configuration, should know which kind of payment processor it needs: secure or fake. This is an issue of application configuration. Create a factory method that detects whether or not a real payment processor is needed:

def create_payment_processor()
  if is_secure_environment():
    return SecurePaymentProcessor()
  else:
    return FakePaymentProcessor()

class SecurePaymentProcessor
  def process_payment(self, payment)
    # process the payment for real

class FakePaymentProcessor
  def process_payment(self, payment)
    # return a fake result for testing purposes

Call it wherever you need a payment processor:

processor = create_payment_processor()

processor.process_payment(payment)

Each payment processor has a process_payment method. One truly processes the payment, the other fakes it. No if statements necessary (except where the payment processor is created). Polymorphism is the answer here. Since you are using Python, you have no need to declare an interface. Just create the two classes and use each of them the same way as you would use the other.

In this way the is no "unwanted" usage of a function.

Answer 5

Just throw an exception (like an InappropriateContextException) and document this behavior. This the object oriented way to go. It does not have to be more complicated or confusing. Anything else is less safe and harder to use.