16

A good quarter of a century ago when I was learning C++, I was taught that interfaces should be forgiving and as far as possible not care about the order that methods were called since the consumer may not have access to the source or documentation in lieu of this.

However, whenever I've mentored junior programmers and senior devs have overheard me, they've reacted with astonishment which has got me wondering whether this was really a thing or if it has just gone out of vogue.

As clear as mud?

Consider an interface with these methods (for creating data files):

OpenFile
SetHeaderString
WriteDataLine
SetTrailerString
CloseFile

Now you could of course just go thru these in order, but say you didn't care about the file name (think a.out) or what header and trailer string were included, you could just call AddDataLine.

A less extreme example might be omitting the headers and trailers.

Yet another might be setting the header and trailer strings before the file has been opened.

Is this a principle of interface design that is recognised or just the POLA way before it was given a name?

N.B. don't get bogged down in the minutiae of this interface, it is just an example for the sake of this question.

Robbie Dee
  • 9,823

2 Answers2

25

One way in which you can stick to the principle of least astonishment is to consider other principles such as ISP and SRP, or even DRY.

In the specific example you've given, the suggestion seems to be that there's a certain dependency of ordering for manipulating the file; but your API controls both the file access and the data format, which smells a bit like a violation of SRP.

Edit/Update: it also suggests that the API itself is asking the user to violate DRY, because they will need to repeat the same steps every time they use the API.

Consider an alternative API where the IO operations are separate from the data operations. and where the API itself 'owns' the ordering:

ContentBuilder

SetHeader( ... )
AddLine( ... )
SetTrailer ( ... )

FileWriter

Open(filename) 
Write(content) throws InvalidContentException
Close()

With the above separation, the ContentBuilder doesn't need to actually "do" anything apart from store the lines/header/trailer (Maybe also a ContentBuilder.Serialize() method which knows the order) . By following other SOLID principles it no longer matters whether you set the header or trailer before or after adding lines, because nothing in the ContentBuilder is actually written to file until its passed to FileWriter.Write.

It also has the added benefit of being a little more flexible; for example, it might be useful to write the content out to a diagnostic logger, or maybe pass it across a network instead of writing it directly to a file.

While designing an API you should also consider error reporting, whether that's a state, a return value, an exception, a callback, or something else. The user of the API will probably expect to be able to programmatically detect any violations of its contracts, or even other errors which it can't control such as file I/O errors.

Ben Cottrell
  • 12,133
12

This is not only about POLA, but also about preventing invalid state as a possible source of bugs.

Let's see how we can provide some constraints to your example without providing a concrete implementation:

First step: Don't allow anything to be called, before a file was opened.

CreateDataFileInterface
  + OpenFile(filename : string) : DataFileInterface

DataFileInterface
  + SetHeaderString(header : string) : void
  + WriteDataLine(data : string) : void
  + SetTrailerString(trailer : string) : void
  + Close() : void

Now it should be obvious that CreateDataFileInterface.OpenFile must be called to retrieve a DataFileInterface instance, where the actual data can be written.

Second step: Make sure, headers and trailers are always set.

CreateDataFileInterface
  + OpenFile(filename : string, header: string, trailer : string) : DataFileInterface

DataFileInterface
  + WriteDataLine(data : string) : void
  + Close() : void

Now you have to provide all required parameters upfront to get a DataFileInterface: filename, header and trailer. If the trailer string is not available until all lines are written, you could also move this parameter to Close() (possibly renaming the method to WriteTrailerAndClose()) so that the file at least cannot be finished without a trailer string.

To reply to the comment:

I like separation of the interface. But I'm inclined to think that your suggestion about enforcement (e.g. WriteTrailerAndClose()) is verging on a violation of SRP. (This is something that I have struggled with on a number of occasions, but your suggestion seems to be a possible example.) How would you respond?

True. I didn't want to concentrate more on the example than necessary to make my point, but it's a good question. In this case I think I would call it Finalize(trailer) and argue that it does not do too much. Writing the trailer and closing are mere implementation details. But if you disagree or have a similar situation where it's different, here's a possible solution:

CreateDataFileInterface
  + OpenFile(filename : string, header : string) : IncompleteDataFileInterface

IncompleteDataFileInterface
  + WriteDataLine(data : string) : void
  + FinalizeWithTrailer(trailer : string) : CompleteDataFileInterface

CompleteDataFileInterface
  + Close()

I wouldn't actually do it for this example but it shows how to carry through the technique consequently.

By the way, I assumed that the methods actually must be called in this order, for example to sequentially write many lines. If this is not required, I would always prefer a builder, as suggested by Ben Cottrel.

Community
  • 1
Fabian Schmengler
  • 569