Separate responsibilities when public methods depend on other public methods in same class?

Question

Recently I hit something like the following setup in our codebase, and I'm wondering if there's a preferred way to do it, to achieve the cleanest separation.

public class FileLoader
{
    public object LoadFile(filename);
    {
        return loadSomethingFromSomewhere(filename)
    }
    
    // new functionality
    public string GetFileVersion(filename);
    {
        return LoadFile(filename).version
    }

    private object loadSomethingFromSomewhere(filename)
    {
        return do().stuff().with(filename)
    }
}

The code already loads a file, and now I want to read some metadata off of it. In the business logic, all this metadata is eventually thrown away, but some parts are now important to introduce.

• GetFileVersion is a pretty simple fix, and doesn't break any compatibility. On the other hand, I dislike the fact that it calls another public function, and then adds an extra step on top.
• Alternatively one could simply tack the version on top of the existing public function, and return it as a <object, string> tuple with the version attached. It doesn't break responsibility now, but it does in theory break the function signature throughout the codebase, and it introduces information that isn't always needed.

What say the design gurus?

Answer 1

You can use

public string GetFileVersion(filename)
{
   return loadSomethingFromSomewhere(filename).version
}

to avoid a public function being called by another in the same class.

Alternatively you can also have a File class with metadata (including version) as its member field.

private File loadSomethingFromSomewhere(filename)
{
    return do().stuff().with(filename)
}

and return file.getObject() and file.getVersion respectively.

Answer 2

To arrive at a good design, you should initially forget how the class is actually implemented. Mentally throw away the non-public functions and the bodies of the public functions and ask yourself, "as a user of this class, does this interface satisfy my needs or do I get too little or too much data back".

For example, your two options reduced to just their public interface

public class FileLoader1
{
    public object LoadFile(filename);
    public string GetFileVersion(filename);
}

public class FileLoader2
{
    public tuple<object, string> LoadFile(filename);
}

From a usage perspective, the first is awkward if the user always wants to retrieve both the version and the file contents. The second is awkward if the typical caller only wants to have either the file data or the version number or doesn't want them at the same time/place.

After you have decided which way the class is most friendly to its users, then you can start to look at the implementation of the various functions. If you find calling public members on yourself distasteful, you can always do something like

public class FileLoader
{
    public object LoadFile(filename);
    {
        return LoadFilePrivate(filename)
    }
    
    // new functionality
    public string GetFileVersion(filename);
    {
        return LoadFilePrivate(filename).version
    }

    private object LoadFilePrivate(filename);
    {
        return loadSomethingFromSomewhere(filename)
    }

    private object loadSomethingFromSomewhere(filename)
    {
        return do().stuff().with(filename)
    }
}