Is there a down side to using a struct as input to a method? [closed]

Question

I'm no expert, but I'm trying to understand if there's a flaw in my logic here. I'm pretty sure it won't take long to have it pointed out if so!

Instead of defining several parameters on a common method, why not simply have a structure as the single input parameter? If each parameter is defined separately and another parameter is added or one is removed, this requires maintenance on every piece of code that calls that method. If a structure was used, then none of the calling code would break, surely? If some code in future required an extra parameter but legacy code didn't, I can add that parameter to the existing list and the legacy code remains untroubled.

As an example: I'm looking at a company where there are multiple websites/projects (to clarify from my original post, which said 'products' and could be misinterpreted as physical products) that need to interface with payment gateways. Depending on the customer location, up to five different payment gateways might be called to process the customer payment.

Currently each one of these multiple products has its own implementation of the payment processing, complete with checking of returned parameters from the gateway. I find this confusing and, well, plain wrong and although there isn't really the time or resource at present to rewrite everything from scratch, I think a relatively quick fix would be to create a new "payments" class project which had a defined set of parameters.

As far as I'm concerned, the front end should only do something as simple as call a method that is "make a payment of x in currency y in country z" and expect a success/fail flag & optional message that clarifies any failure in return. This way, every product the company has will share the same payment processing.

Now parameter wise, currently the requirement for input would only be amount, currency & country (no, you can't always infer currency based on customer location before someone suggests it!). And for a return it would only be a Boolean success/fail and a message explaining the reason for failure. However it's possible that extra parameters will be required - both for in and out - in future and without wanting to break the existing calls to the method, it would appear sensible to simply use a simple structure, e.g.:

 struct inParams
 {
     public decimal amount;
     public string currency;
     public string country;
 };

 struct outParams
 {
     public bool success;
     public string message;
 };

    public outParams makePayment(inParams ip)
    {
        //...code goes here
    }

Is this a "Bad Thing" to do code-wise or is it acceptable practice? If anyone thinks that it is better to have the parameters defined in the method signature individually, can you please explain why that would be better than my suggestion to use a simple structure please?

---

In response to "oɔɯǝɹ"

I don't really see that the issues you've raised are that serious though.

For example a sample call would be something like:

       PaymentProcessor pp = new PaymentProcessor();
       ppInParams ppIn = new ppInParams();
       ppIn.amount = 100;
       ppIn.currency = "USD";
       ppIn.country = "USA";

       ppOutParams ppOut = pp.makePayment(ppIn);
       if (!ppOut.success)
       {
           //check error message, handle error, display message
       }
       else
       {
           //display confirmation message to user, update account log
       }

Your point: This leads to not very discoverable code is hard to justify in the face of such simplicity as shown above, surely?

Users need to add extra boiler plate code just to instantiate your parameter struct. - well, yes, but it's not a major ballache to add a line of code for each parameter, is it?

You will need to manage heaps of parameter structs. Two for every method. -How are you even going to name them? -In case you're thinming, I know, I'll just reuse them, then you've made your problem worse. Whats gonna happen when a single method using a struct need another parameter?

I don't see the problem here either. I have a single "paymentProcessor" class in which I have "ppInParams" and "ppOutParams" and they are contained within the class itself - to me, a perfectly logical place to put them as they relate directly to that class.

Your final point : And in particular, your return type struct smells funny to me. Do you expect that every caller will inspect the boolean result? And what should i do with the message? Log it? show it? Have you not heard of structured exception handling? seems bizarre to me and I don't understand the point you're trying to make. One million percent YES I expect every caller to check the boolean response as that's what tells them if the payment was a success or not! This is the whole point of simplifying the call like this surely? As the error handling is different depending on which web site has called the "makepayment" method, then again it makes perfect sense to me to handle the error back in the calling code.

Answer 1

Products should not include payment processing. Payment processing does not describe an attribute or action of Product it describes an action on a Product. You likely need a Payment object which can be loosely based on your inParams struct:

class Payment {
    public decimal Amount { get; set; }
    public string Currency { get; set; }
}

The Country property is interesting. If knowing where the order was placed is important the Order came from a single location so Country should belong to Order. However, if knowing which country each payment came from is important then Country should belong to Payment. That is up to you. I included Country in Order.

You will also need an Order object to group the Payment and Products. Notice the Order object accepts an interface IPaymentProcessor as a constructor argument. You can implement different payment processing behavior by passing a different implementation of the IPaymentProcessor interface into the Order object. You can apply your business logic to figure out which concrete implementation should be sent to the Order constructor. The important part of this is you will never need to go alter your Order or Payment classes to change the payment processing behavior.

class Order {
    public string Country { get; set; }
    public ICollection<Payment> Payments { get; set; }
    public ICollection<Product> Products { get; set; }
    private IPaymentProcessor paymentProcessor;

    public Order(IPaymentProcessor paymentProcessor) {
        this.paymentProcessor = paymentProcessor;
    }

    void MakePayment(Payment payment) {
        this.paymentProcessor.ProcessPayment(payment);
    }
}

If you will never accept multiple Payments on an Order you can change the ICollection<Payment> Payments to Payment Payment.

IPaymentProcessor interface:

interface IPaymentProcessor {
    void ProcessPayment(Payment payment);
}

Remember structs are value types and will be copied when they are passed as a parameter to a method. Classes are references types and only the reference (32 or 64 bit value based on the computer's word size) is passed by value to a method. It looks like a class is better in the situation, especially if you anticipate the object to grow.

UPDATE AFTER COMMENT:

Well that certainly changes how I read your question! I think much of the answer still applies so I will leave it. Using the logic above I would implement an OrderProcessor Service that would accept Order objects and invoke their ProcessPayment method (and whatever other logic is required). You still inject the IPaymentProcessor interface based on the rules of the system (where the order originated). Alternatively, if chunks of the order process are similar then consider making IPaymentProcessor into a base class PaymentProcessor where you can override specific behaviors in the base class when needed. There are lots of ways to implement this but the main idea I am trying to convey is this is an excellent case for polymorphism and this is just an example of how you could construct a basic architecture using that principle.