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

Raymond Chen

Raymond

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?

Raymond Chen
Raymond Chen

Follow Raymond   

32 Comments
Avatar
Damien Knapman 2019-03-18 07:27:57
Aplogies if I mix C# and C++ terminology but I'd think you ensure that the class has a delegate it can call when the new object is created but you move the creation of your new object into this class'es destructor. So you only get the new object when the old has been destroyed. (I'm sure this is all kinds of levels of evil and may even not work for non-obvious-to-me reasons at the moment.
Avatar
Peter Ruderman 2019-03-18 07:40:38
I'm a little hesitant to suggest this (since someone might go and use it), but you could use a ref-qualifier.  Something like this: class Step1 { public:   Step2 DoIt() &&;};   So calling DoIt() gives you a Step2 object and leaves Step1 in a moved from state (where presumably you can no longer do anything with it).  But honestly, I think the best answer is to not require that your clients call methods in a particular order.
Avatar
Stephan Leclercq 2019-03-18 08:11:59
Use inversion of control. Instead of having the client code assign properties and call methods on the PhotoImporter, have the client register callback functions (or async events...) to the importer, then call the single PhotoImporter::import() method. This in turn performs the operations in the correct way and invoke the callbacks in the appropriate order. The callbacks cannot "do" something, they can just tell what they want to be done.
Avatar
A P² 2019-03-18 10:30:28
I don't know about C#. In a language with dependent types, it's certainly possible: http://docs.idris-lang.org/en/latest/st/introduction.html
Avatar
Ari Kalish 2019-03-18 11:03:56
We've handled this situation in our C# apps through the Builder Pattern.Create a series of interfaces that define the individual steps as methods that return the interface that defines the next step. Then call them through a builder that internally constructs an object that implements all of the interfaces.  Something like this: <pre> public interface IStage0{     IStage1 MoveOnToStage1(); } public interface IStage1 {    IStage2 MoveOnToStage2(); } public interface IStage2{     IStage3 MoveOnToStage3(); } public interface IStage3{     void Done(); } internal class Foo : IStage0, IStage1, IStage2, IStage3 {     public Foo(InitialData data)     {        // set internal state    }     public IStage1 MoveOnToStage1()     {         // do work         return this;     }         public IStage2 MoveOnToStage2()     {         // do work         return this;     }         public IStage3 MoveOnToStage3()     {         // do work         return this;     }         public void Done()     {         // do work     } } public class FooBuilder{     public FooBuilder();     public IStage0 GetInitialObject(InitialData data) // this could also be a static method on Foo if you want to make Foo public and give it a private constructor.     {          return new Foo(data);     } } And to call it:   new FooBuilder().GetInitialObject(initialData).MoveOnToStage1().MoveOnToStage2().MoveOnToStage3().Done(); </pre>
Avatar
Lorenzo N. 2019-03-18 13:48:23
You could use a modified passkey idiom. Each function would return a non-copiable key, that has to be moved into the next function: class Camera;class PhotoImporter;struct DiscoveredPhotos{ friend class PhotoImporter;private: DiscoveredPhotos(){}};struct SelectedPhotos{ friend class PhotoImporter;private: SelectedPhotos() {}};class PhotoImporter{public: PhotoImporter(const Camera& camera); DiscoveredPhotos DiscoverPhotos(); SelectedPhotos SelectPhotos(DiscoveredPhotos&& discovered_photos); void Download(SelectedPhotos&& selected_photos);};
Avatar
Rob Eady 2019-03-18 15:47:37
> 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 system.   // Edit: the formatting of comments here is awful. Everything gets ripped out, even paragraphs and line breaks!
Avatar
cheong00 2019-03-18 18:17:21
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.
Avatar
Anthony Griesel 2019-03-18 19:04:55
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)                .SetSelectPhotosFunction(x => x.Name.Equals("1.jpeg"))                .Build(camera);            importer.Download(); // only public method on importer. Configuration was done by builder.       public class PhotoImporter    {        private readonly Camera camera;        private bool newPhotosOnly;        private string downloadPath;        private bool reNumberPhotos;        private bool deleteOriginals;        private PhotoImporter(Camera camera)//private constructor forces use of Builder.        {            this.camera = camera;        }        public void Download()        {            // state has been managed by builder.         }        public class Builder // nestled class has access to private fields/methods        {            private bool newPhotosOnly;            private string downloadPath;            private bool reNumberPhotos;            private bool deleteOriginals;            private Func<Photo, bool> selectPhotosFunction;            public Builder SetFindOnlyNewPhotos(bool newPhotosOnly)            {                this.newPhotosOnly = newPhotosOnly;                return this; // builder methods always return this instance so further configuration is possible.            }            public Builder SetDownloadFolder(string downloadPath)            {                this.downloadPath = downloadPath;                return this;            }            public Builder SetRenumberPhotos(bool reNumberPhotos)            {                this.reNumberPhotos = reNumberPhotos;                return this;            }            public Builder SetDeleteOriginalsAfterDownload(bool deleteOriginals)            {                this.deleteOriginals = deleteOriginals;                return this;            }            public Builder SetSelectPhotosFunction(Func<Photo, bool> selectPhotosFunction)            {                this.selectPhotosFunction = selectPhotosFunction;                return this;            }            public PhotoImporter Build(Camera camera)            {                PhotoImporter importer = new PhotoImporter(camera);                importer.newPhotosOnly = this.newPhotosOnly;                importer.reNumberPhotos = this.reNumberPhotos;                importer.deleteOriginals = this.deleteOriginals;                if(string.IsNullOrWhiteSpace(this.downloadPath))                    throw new InvalidOperationException("No path specified.");                if(!Directory.Exists(this.downloadPath))                    throw new InvalidOperationException("Download Path does not exist.");                importer.downloadPath = this.downloadPath;                                if (this.selectPhotosFunction == null)                    throw new InvalidOperationException("Photo Selection function must be set.");                                return importer;            }        }    }
Avatar
anonymous 2019-03-19 00:54:11
This comment has been deleted.
Avatar
Accel229 2019-03-19 01:52:29
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 manually), etc, etc, etc. None of this is Raymond's fault, but since the braindead decision to migrate isn't my fault either and I have been a reader since the very beginning, I am going to say that the migration was stupid, that the old site was much better, and request that someone wakes up to this and maybe, I don't know, considers doing something about it. The awful performance and awful qualities of the new site make visiting this blog unpleasant. Yes, I submitted this as a feedback as well and if you think the same, I encourage you to do it, too. Shutting up now.
Avatar
Michael J Smith 2019-03-19 04:25:26
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.
Avatar
Steven Wolf 2019-03-19 10:45:10
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, this-state-instance is invalidated in terms of reusing it (or maybe it is simply marked as dead the second it gives up the next-state instance).I don't see how you can enforce any of the backwards blocking at compile time, only at runtime, unless as some posters have mentioned - using move semantics to consume the previous state instance when generating the new-state-instance.Although - that just seems like a fine solution anyway.  A is consumed to produce B is consumed to produce C ...
Avatar
David Muller 2019-03-19 16:36:22
Avatar
Christof Donat 2019-03-21 02:53:05
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 value can only be used on the right hand side of a `|`-operator with the return value of the former `|`-operator on the left hand side, etc. All the left hand side objects will be passed as rvalue references, so they can not be reused as well.
Avatar
Arnaud Forgiel 2019-03-21 08:33:18
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.
Avatar
Ajith S. 2019-03-27 14:23:14
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.