March 18th, 2019

How do I design a class so that methods must be called in a certain order?

Suppose you have a class with some methods, and the methods must be called in a certain order. For example, suppose your class is for importing photos from a camera. First, you have to specify which camera you are importing from. Then you discover the pictures. Then you have to select which pictures you want to import, and set the options for how you want the download to occur. And finally, you download them.

class PhotoImporter
{
public:
 PhotoImporter(const Camera& camera);

 // Optionally configure the discovery.
 void SetFindOnlyNewPhotos(bool newPhotosOnly);

 // Call this first to get some photos.
 std::vector<Photo> DiscoverPhotos();

 // Then decide which photos you want to download by calling
 // this for each photo you want to download.
 void SelectPhoto(Photo photo);
 void SelectAllDiscoveredPhotos();

 // Configure the download. You must set a download folder.
 // The other settings default to false.
 void SetDownloadFolder(const path& downloadFolder);
 void SetRenumberPhotos(bool renumber);
 void SetDeleteOriginalsAfterDownload(bool deleteOriginals);

 // And then download them.
 void Download();
};

The problem is that there is nothing preventing the caller from calling the methods out of order or omitting some methods altogether.

void confused()
{
 PhotoImporter importer(mainCamera);
 importer.SelectAllDiscoveredPhotos();
 importer.Download();
 importer.SetRenumberPhotos(true);
 importer.DiscoverPhotos();
 // never specified a download folder
}

One trick for making it harder to call the methods in the wrong order is to represent each state of the import process as a separate class.

class PhotoImporter
{
public:
 PhotoImporter(const Camera& camera);

 // Optionally configure the discovery.
 void SetFindOnlyNewPhotos(bool newPhotosOnly);

 // Call this first to get some photos.
 DiscoveredPhotos DiscoverPhotos();
};

class DiscoveredPhotos
{
public:
 // Not publically constructible.

 const std::vector<Photo>& GetPhotos();

 // Decide which photos you want to download by calling
 // this for each photo you want to download.
 void SelectPhoto(Photo photo);
 void SelectAllDiscoveredPhotos();

 // Configure the download. You must set a download folder.
 // The other settings default to false.
 void SetDownloadFolder(const path& downloadFolder);
 void SetRenumberPhotos(bool renumber);
 void SetDeleteOriginalsAfterDownload(bool deleteOriginals);

 // And then download them.
 void Download();
}

Breaking it up this way means that it is impossible to call Download before calling Discover­Photos, because in order to download the photos, you need to get a Discovered­Photos object, and the only way to get one of those is by calling Discover­Photos.

And then there’s the issue of requiring a download folder. For that, we could make the mandatory portion an explicit parameter to the Download method. We used this trick when we required the Camera to be passed to the Photo­Importer constructor.

class DiscoveredPhotos
{
public:
 ...
 void Download(const path& downloadFolder);
}

And to ensure that people don’t try to do things like call Set­Renumber­Photos after Download, you could put all of the download options into an options class.

class DownloadOptions
{
public:
 void SetRenumberPhotos(bool renumber);
 void SetDeleteOriginalsAfterDownload(bool deleteOriginals);
};

class DiscoveredPhotos
{
public:
 // Not publically constructible.

 const std::vector<Photo>& GetPhotos();

 // Decide which photos you want to download by calling
 // this for each photo you want to download.
 void SelectPhoto(Photo photo);
 void SelectAllDiscoveredPhotos();

 // And then download them.
 void Download(const path& downloadFolder,
               const std::optional<DownloadOptions>& options = std::nullopt);
}

While this technique forces the programmer to satisfy prerequisites before calling a method, it doesn’t prevent the programmer from trying to go backwards. For example, after calling Download, the programmer could go back and select some more photos and then call Download a second time.

If you want to disallow that, then I’m stumped. I can’t think of something that prevents an object from being used after a particular method is called, with enforcement at compile time. Maybe you can think of something?

Topics
Code

Author

Raymond has been involved in the evolution of Windows for more than 30 years. In 2003, he began a Web site known as The Old New Thing which has grown in popularity far beyond his wildest imagination, a development which still gives him the heebie-jeebies. The Web site spawned a book, coincidentally also titled The Old New Thing (Addison Wesley 2007). He occasionally appears on the Windows Dev Docs Twitter account to tell stories which convey no useful information.

34 comments

Discussion is closed. Login to edit/delete existing comments.

  • Ajith S.

    Shouldn’t DiscoveredPhotos::GetPhotos() return by value instead of reference? Not sure if that’s a typo or if I’m missing something in the example.

  • Arnaud ForgielMicrosoft employee

    Another approach would be to use a Finite State Machine with a method passing the “message” of the next step to be performed, a bit like:bool Perform( const char * message, …);Using va_arg, you get the appropriate parameters and the FSM ensure the user respects the proper import order.

    • Raymond ChenMicrosoft employee Author

      But how do you enforce this at compile time?

  • Christof Donat

    I'd try with functions, that take a rvalue reference to the object, that should not be used any more after the call. So the code could look like
    ```photo_importer::download(photo_importer::discover_photos(photo_importer::chose_camera(main_camera)), target_dir); ```
    Since the return value of chose_camera and discover_photos will be moved in, there is no way to use these objects afterwards.
    You might also consider to overload the |-Operator like the ranges-TS does:
    ```photo_importer::chose_camera(main_camera) | photo_importer::discover_photos() | photo_importer::download(target_dir);```
    Then the return value of `chose_camera()` can only be used in a `|`-opreator on the left hand side with the return type of `discover_photos()` on the right hand sinde. The return...

    Read more
    • Raymond ChenMicrosoft employee Author

      But there is nothing that prevents you at compile time from trying to use a moved-from object.

      • Hristo Hristov

        If a solution isn’t obvious, wouldn’t that be an unnecessary complication? What about implementing the limitation via asserts?

      • Raymond ChenMicrosoft employee Author

        The goal here is to make it impossible even to write the code that does the wrong thing. Sure, you could do all this with asserts, but those aren’t checked until runtime, and it might be in a code path that is rarely-triggered and eluded testing.

      • Hristo Hristov

        A good point. Maybe in that case we just need new language facilities to enable that pattern?

  • Steven Wolf

    Why not just use a state machine within the class instances?Only allow appropriate state-transitions.  A client that violates them gets a clear indication of what went wrong, albeit at runtime.Similarly, one could block clients from "going backwards" by setting a state in the previous state-class (from your example Raymond - of a class for each state accessible only from the previous state) - by having each one supply a back-ref or a shared pointer to a live-state flag that simply isn't cleared unilt the subsequent instance is closed.  Some sort of chaining so that until the supplied next-state-instance is done,...

    Read more
  • Michael J Smith

    One strategy would be to make all of your functional methods private but expose function pointers.  Set the function pointers only when it is appropriate.  So the constructor will initialise the pointer to phase1() and NULL the others.  When phase1() completes it sets the pointer to phase2() and NULLs the phase1() pointer.  etc.
    Of course you could more easily permit the programmer to call any method, but return fail status unless the object is in the correct state.

  • Accel229

    I already commented on the new site being awful in the post about the migration, but the new site is really so terrible and gets in the way so much, that I will comment one more time - and then shut up. The new site is (a) very slow (8 seconds to open a page is complete bonkers), (b) breaks comment formatting (see comments in this post for example), and (c) lost old comments which were 80% of the value of the blog. It also looks worse on mobile, lost archive links (yes, I know that you can type URLs...

    Read more
    • Henry Skoglund

      Agreed, this blog is more or less equivalent to a brand new blog. Many times I’ve gone here thanks to Hacker News, for example https://news.ycombinator.com/item?id=4781372
      But of course the links in that post are now defunct. If Hacker News can preserve contents from 7 years back, why cannot Microsoft?

      • Accel229

        I'll say more - I have no idea why the heck they decided to migrate at all. Just why? I don't understand it. I guess they did have some reason, but I legitimately have no clue as to what it is. It cannot be better user experience, because it became worse, not better, visibly and on multiple fronts. I am not being facetious here. If the reason for the migration *was* better user experience - then why nobody cared enough to test what it is going to be??? Because if they did test it, they'd immediately see that yeah, the...

        Read more
  • Anthony Griesel

    Temporal coupling like this should be avoided if possible, but oftentimes its hard to do in real world with the clock ticking. One way we have mitigated the issue is by using fluent class builders. They do not bloat your class's constructor, and allow for lots of flexibility, it still constrains you to having only 1 public method on your class. Everyhing else had to be setup by builder/factory. You can call the builder methods in any order, but to get an configured object, you have to call build on factory.
     
    PhotoImporter importer = new PhotoImporter.Builder()                .SetFindOnlyNewPhotos(true)                .SetDownloadFolder(Path.GetTempPath())                .SetRenumberPhotos(true)               ...

    Read more
  • cheong00

    I would have create a static method .CreatePhotoImporter(), with parameter that is an object that wraps all required parameters in, then calls these “must be called in certain order” methods for the user before returning the object. Those methods could be changed to private if there’s no good reason to expose them.
    Essentially it’s similar idea to what “parameter parsers” do.

  • Rob Eady

    > I can’t think of something that prevents an object from being used after a particular method is called, with enforcement at compile time 
     
     -- Rust move semantics offer a neat solution here. A function like download could accept its DiscoveredPhotos argument as a value. If DiscoveredPhotos is not a `Copy`able type, this means transfering ownership, preventing the previous owner from using the value afterwards. This is quite commonly applied as part of the builder pattern, where constructing the final type can consume the builder by taking it as a value.
     
    In a sense Rust implements an affine type...

    Read more