Pass object twice to same method or consolidate with combined interface?

Question

I have a method that creates a data file after talking to a digital board:

CreateDataFile(IFileAccess boardFileAccess, IMeasurer boardMeasurer)

Here boardFileAccess and boardMeasurer are the same instance of a Board object that implements both IFileAccess and IMeasurer. IMeasurer is used in this case for a single method that will set one pin on the board active to make a simple measurement. The data from this measurement is then stored locally on the board using IFileAccess. Board is located in a separate project.

I've come to the conclusion that CreateDataFile is doing one thing by making a quick measurement and then storing the data, and doing both in the same method is more intuitive for someone else using this code then having to make a measurement and write to a file as separate method calls.

To me, it seems awkward to pass the same object to a method twice. I've considered making a local interface IDataFileCreator that will extend IFileAccess and IMeasurer and then have an implementation containing a Board instance that will just call the required Board methods. Considering that the same board object would always be used for measurement and file writing, is it a bad practice to pass the same object to a method twice? If so, is using a local interface and implementation an appropriate solution?

Answer 1

No, this is perfectly fine. It merely means that the API is over-engineered with regards to your current application.

But that doesn't prove that there will never a use case in which the data source and the measurer are different. The point of an API is to offer the application programmer possibilities, not all of which will be used. You should not artificially restrict what API users can do unless it complicates the API so that the net understandability goes down.

Answer 2

Agree with @KilianFoth's answer that this is perfectly fine.

Still, if you want, you can create a method that takes a single object that implements both interfaces:

public object CreateDataFile<T_BoardInterface>(
             T_BoardInterface boardInterface
    )
    where T_BoardInterface : IFileAccess, IMeasurer
{
    return CreateDataFile(
                boardInterface
            ,   boardInterface
        );
}

There's no general reason that arguments need to be different objects, and if a method did require arguments to be different, that'd be a special requirement its contract should make clear.

Answer 3

I've come to the conclusion that CreateDataFile is doing one thing by making a quick measurement and then storing the data, and doing both in the same method is more intuitive for someone else using this code then having to make a measurement and write to a file as separate method calls.

I think this is your problem, actually. The method is not doing one thing. It's performing two, distinct operations that involve I/O to different devices, both of which it's off-loading to other objects:

• Fetch a measurement
• Save that result to a file somewhere

These are two different I/O operations. Notably, the first one does not mutate the file system in any way.

In fact, we should note that there's an implied middle step:

• Fetch a measurement
• Serialize the measurement into a known format
• Save the serialized measurement to a file

Your API should provide each of these separately in some form. How do you know a caller will not want to take a measurement without storing it anywhere? How do you know they won't want to obtain a measurement from another source? How do you know they won't want to store it somewhere other than the device? There is good reason to decouple the operations. At a bare minimum, each individual piece should be available to any caller. I should not be forced to write the measurement to a file if my use case does not call for it.

As an example, you might separate the operations like this.

IMeasurer has a way to fetch the measurement:

public interface IMeasurer
{
    IMeasurement Measure(int someInput);
}

Your measurement type might just be something simple, like a string or decimal. I'm not insisting you need an interface or class for it, but it makes the example here more general.

IFileAccess has some method for saving files:

interface IFileAccess
{
    void SaveFile(string fileContents);
}

Then you need a way of serializing a measurement. Build that into the class or interface representing a measurement, or have a utility method:

interface IMeasurement
{
    // As part of the type
    string Serialize();
}

// Utility method. Makes more sense if the measurement is not a custom type.
public static string SerializeMeasurement(IMeasurement m)
{
    return ...
}

It's not clear whether you have this serialization operation separated out yet.

This sort of separation improves your API. It lets the caller decide what they need and when, rather than forcing your preconceived ideas about what I/O to perform. Callers should have the control to perform any valid operation, whether you think it's useful or not.

Once you have separate implementations for each operation, your CreateDataFile method becomes just a shorthand for

fileAccess.SaveFile(SerializeMeasurement(measurer.Measure()));

Notably, your method adds very little value once you've done all this. The above line of code is not difficult for your callers to use directly, and your method is purely for convenience at most. It should be and is something optional. And that is the correct way for the API to behave.

---

Once all the relevant parts are factored out and we've acknowledged that the method is just a convenience, we need to rephrase your question:

What would be the most common use case for your callers?

If the entire point is to make the typical use case of measuring from and writing to the same board slightly more convenient, then it makes perfect sense to just make it available on the Board class directly:

public class Board : IMeasurer, IFileAccess
{
    // Interface methods...

    /// <summary>
    /// Convenience method to measure and immediate record measurement in
    /// default location.
    /// </summary>
    public void ReadAndSaveMeasurement()
    {
        this.SaveFile(SerializeMeasurement(this.Measure()));
    }
}

If this doesn't improve convenience, then I wouldn't bother with the method at all.

---

This being a convenience method brings up one other question.

Should the IFileAccess interface know about the measurement type and how to serialize it? If so, you could add a method to IFileAccess:

interface IFileAccess
{
    void SaveFile(string fileContents);
    void SaveMeasurement(IMeasurement m);
}

Now callers just do this:

fileAccess.SaveFile(measurer.Measure());

which is just as short and probably more clear than your convenience method as conceived in the question.

Answer 4

The consuming client should not have to deal with a pair of items when a single item suffices.  In your case, they almost don't, until the invocation of CreateDataFile.

The potential solution that you suggest is to create a combined derived interface.  However, this approach requires a single object that implements both interfaces, which is rather constraining, arguably a leaky abstraction in that it is basically customized to a particular implementation.  Consider how complicated it would be if someone wanted to implement the two interfaces in separate objects: they'd have to proxy all the methods in one of the interfaces in order to forward to the other object.  (FWIW, another option is to just merge the interfaces rather than requiring one object has to implement two interfaces via a derived interface.)

However, another approach that is less constraining on/dictating to the implementation is that IFileAccess is paired with an IMeasurer in composition, so that one of them is bound to and references the other.  (This somewhat increases the abstraction of one of them since it now represents the pairing as well.)  Then CreateDataFile could take only one of the references, say IFileAccess, and still obtain the other as needed.  Your current implementation as an object that implements both interfaces would simply return this; for the composition reference, here the getter for IMeasurer in IFileAccess.

If the pairing turns out to be false at some point in development, which is to say sometimes a different measurer is used with the same file access, then you can do this same pairing but at a higher level instead, meaning the additional interface introduced would not be a derived interface, but rather an interface that has two getters, pairing a file access and measurer together via composition rather than derivation.  The consuming client then has one item to concern themselves with as long as the pairing holds, and individual objects to deal with (to compose new pairings) when necessary.

---

On another note, I might ask who owns CreateDataFile, and the question goes to who this third party is.  We already have some consuming client that invokes CreateDataFile, the owning object/class of CreateDataFile, and the IFileAccess and IMeasurer.  Sometimes when we take a larger view of the context, alternate, sometimes better, organizations can appear.  Hard to do here since the context is incomplete, so just food for thought.

Answer 5

Some have brought up that CreateDataFile is doing too much. I might suggest that instead Board is doing too much, as accessing a file seems like a separate concern from the rest of the board.

However, if we assume that this is not a mistake, the bigger issue is that the interface should be defined by the client, in this case CreateDataFile.

The Interface Segregation Principle states that the client should not have to depend on more of an interface than what it needs. Borrowing the phrase from this other answer, this can be paraphrased as "an interface is defined by what the client needs."

Now, it is possible to compose this client-specific interface using IFileAccess and IMeasurer as other answers suggest, but ultimately, this client should have an interface tailor-made for it.