0

I inherited a little over 1000 lines of spaghetti code. I can decompose it into a dozen methods that each create a FinancialTransactionObject but take different dates, amounts and other parameters to create their transaction.

My gut tells me to make each method its own class. So I can have a dozen classes inherit from a base class or interface with 1 method:

abstract FinancialTransactionObject Calculate();

and move the parameters either to constructors or make them public properties. Constructors mean I cannot reuse my instance and have to create a new object each time. Properties mean the consuming code can forget to set them. Leaving parameters on each method just gives me a dozen methods in separate files and doesn't feel object oriented.

It seems like a variation of this problem comes up often. Is there a good, consistent, industry wide design pattern to handle it?

Brad
  • 1,360
  • 4
  • 18
  • 27
  • I would say: A single class. A bunch of Static functions returning FinancialTransactionObject. Same name, overloaded parameters. – Christopher Mar 14 '18 at 15:47
  • If they all return the same class (and not something inherited), why aren’t they constructors? If they return different objects, they can still be just constructors but for different classes. And if the point is to hide what object comes out, I would just keep them as methods in the same class. – Sami Kuhmonen Mar 14 '18 at 15:47
  • Why is reusing the instance important to you? Do those classes hold resources which are expensive to create / open? – Fildor Mar 14 '18 at 15:52
  • Are there any common properties at all or just a `Calculate()` method? – itsme86 Mar 14 '18 at 15:56
  • 1
    Notice that what @Christopher is suggesting is the *Factory pattern* and it does sound like you want that – Camilo Terevinto Mar 14 '18 at 15:58
  • However if your abstract class has only abstract methods it should instead be an interface. – Steve Harris Mar 14 '18 at 16:14
  • One possibility is the Strategy Pattern. See [Factory method with DI and IoC](https://stackoverflow.com/a/31971691) and [this answer](https://stackoverflow.com/a/32415954). – NightOwl888 Mar 14 '18 at 16:49

1 Answers1

0

This really sounds to me like a Visitor pattern.

So basic idea of the visitor pattern is to change the behavior dynamically according to the type of implementation.

You will need 2 main things:

  1. IVisitable with an Accept method having the IVisitor as the parameter.
  2. IVisitor with many Visit methods for each implementation of IVisitable

I'll start from number 2. This (in your case) will be an interface, that has all the implementations of your calculate method (for each of the classes that you mentioned you want to create). Lets call your Visitor and Visit methods Calculator and Calculate. Then you will have:

interface ICalculator
{
    FinancialTransactionObject Calculate(Element1 element);
    FinancialTransactionObject Calculate(Element2 element);
    FinancialTransactionObject Calculate(Element3 element);
    .
    .
}

Then the first part - all the element classes, will inherit a base class Element, which will implement IVisitable. And then something like:

abstract class Element
{
    public FinancialTransactionObject result { get; private set; }

    protected void Accept(ICalculator calculator)
    {
        this.result = calculator.Calculate(this);
    }
}

You will end up with something like:

var calculator = new Calculator(); //this is the class that implements the interface
var el1 = new Element1();
var el2 = new Element2();

el1.Accept(calculator);
el2.Accept(calculator);

Then of course based on the implementation your el objects, will have their result values.

I'm not sure that this is the best solution for your problem (if there is such at all), but seems to me like the one you are looking for. Hope that this helps.

PS: I was thinking naming IVisitable to ICalculatable but it sounded weird :)

m3n7alsnak3
  • 3,026
  • 1
  • 15
  • 24