How to warn other programmers of class implementation

Question

I'm writing classes that "must be used in a specific way" (I guess all classes must...).

For example, I create the fooManager class, which requires a call to, say, Initialize(string,string). And, to push the example a little further, the class would be useless if we don't listen to its ThisHappened action.

My point is, the class I'm writing requires method calls. But it will compile just fine if you don't call those methods and will end up with an empty new FooManager. At some point, it will either not work, or maybe crash, depending on the class and what it does. The programmer that implements my class would obviously look inside it and realize "Oh, I didn't call Initialize!", and it'd be fine.

But I don't like that. What I would ideally want is the code to NOT compile if the method wasn't called; I'm guessing that's simply not possible. Or something that would immediately be visible and clear.

I find myself troubled by the current approach I have here, which is the following:

Add a private Boolean value in the class, and check everywhere necessary if the class is initialized ; if not, I will throw an exception saying "The class wasn't initialized, are you sure you're calling .Initialize(string,string)?".

I'm kind of okay with that approach, but it leads to a lot of code that is compiled and, in the end, not necessary to the end user.

Also, it's sometimes even more code when there are more methods than just an Initiliaze to call. I try to keep my classes with not too many public methods/actions, but that's not solving the problem, just keeping it reasonable.

What I'm looking for here is:

• Is my approach correct?
• Is there a better one?
• What do you guys do/advise?
• Am I trying to solve a non-issue? I've been told by colleagues it's the programmer's to check the class before trying to use it. I respectfully disagree, but that's another matter I believe.

---

Put it simply, I'm trying to figure out a way to never forget to implement calls when that class is reused later, or by someone else.

CLARIFICATIONS :

To clarify many questions here :

• I'm definitely NOT only talking about the Initialisation part of a class, but rather it's whole lifetime. Prevent colleagues to call a method twice, making sure they call X before Y, etc. Anything that would end up being mandatory and in documentation, but that I would like in code and as simple and small as possible. I really liked the idea of Asserts, though I'm quite sure I'll need to mix some other ideas as Asserts will not always be possible.

• I'm using the C# language ! How did I not mention that?! I'm in a Xamarin environment and building mobile apps usually using about 6 to 9 projects in a solution, including PCL's, iOS, Android and Windows projects. I've been a developer for about a year and a half (school and work combined), hence my sometimes ridiculous statements\questions. All that is probably irrelevant here, but too much information isn't always a bad thing.

• I can't always put everything that is mandatory in the constructor, due to platform restrictions and the use of Dependency Injection, having parameters other than Interfaces is off the table. Or maybe my knowledge is not sufficient, which is highly possible. Most of the time it's not an Initialisation issue, but more

how can I make sure he registered to that event ?

how can I make sure he didn't forget to "stop the process at some point"

Here I remember an Ad fetching class. As long as the view where the Ad is visible is visible, the class would fetch a new ad every minute. That class needs a view when constructed where it can display the Ad, that could go in a parameter obviously. But once the view is gone, StopFetching() must be called. Otherwise the class would keep fetching ads for a view that isn't even there, and that's bad.

Also, that class has events that must bé listened to, like "AdClicked" for example. Everything works fine if not listened to, but we lose tracking of analytics there if taps aren't registered. The Ad still works though, so the user and developer won't see a difference, and analytics will just have wrong data. That needs to be, avoided, but I'm not sure how developer can know they must register to the tao event. That is a simplified example though, but the idea is there, "make sure he uses the public Action that is available" and at the right times of course!

Answer 1

In such cases, it is best to use the type system of your language to help you with proper initialization. How can we prevent a FooManager from being used without being initialized? By preventing a FooManager from being created without the necessary information to properly initialize it. In particular, all initialization is the responsibility of the constructor. You should never let your constructor create an object in an illegal state.

But callers need to construct a FooManager before they can initialize it, e.g. because the FooManager is passed around as a dependency.

Don't create a FooManager if you don't have one. What you can do instead is pass an object around that lets you retrieve a fully constructed FooManager, but only with the initialization information. (In functional-programming speak, I'm suggesting you use partial application for the constructor.) E.g.:

ctorArgs = ...;
getFooManager = (initInfo) -> new FooManager(ctorArgs, initInfo);
...
getFooManager(myInitInfo).fooMethod();

The problem with this is that you have to supply the init info every time you access the FooManager.

If it's necessary in your language, you can wrap the getFooManager() operation in a factory-like or builder-like class.

I really want to do runtime checks that the initialize() method was called, rather than using a type-system-level solution.

It is possible to find a compromise. We create a wrapper class MaybeInitializedFooManager that has a get() method that returns the FooManager, but throws if the FooManager wasn't fully initialized. This only works if the initialization is done through the wrapper, or if there is a FooManager#isInitialized() method.

class MaybeInitializedFooManager {
  private final FooManager fooManager;

  public MaybeInitializedFooManager(CtorArgs ctorArgs) {
    fooManager = new FooManager(ctorArgs);
  }

  public FooManager initialize(InitArgs initArgs) {
    fooManager.initialize(initArgs);
    return fooManager;
  }

  public FooManager get() {
    if (fooManager.isInitialized()) return fooManager;
    throw ...;
  }
}

I don't want to change the API of my class.

In that case, you'll want to avoid the if (!initialized) throw; conditionals in each and every method. Fortunately, there is a simple pattern to solve this.

The object you provide to users is just an empty shell that delegates all calls to an implementation object. By default, the implementation object throws an error for each method that it wasn't initialized. However, the initialize() method replaces the implementation object with a fully-constructed object.

class FooManager {
  private CtorArgs ctorArgs;
  private Impl impl;

  public FooManager(CtorArgs ctorArgs) {
    this.ctorArgs = ctorArgs;
    this.impl = new UninitializedImpl();
  }

  public void initialize(InitArgs initArgs) {
    impl = new MainImpl(ctorArgs, initArgs);
  }

  public X foo() { return impl.foo(); }
  public Y bar() { return impl.bar(); }
}

interface Impl {
  X foo();
  Y bar();
}

class UninitializedImpl implements Impl {
  public X foo() { throw ...; }
  public Y bar() { throw ...; }
}

class MainImpl implements Impl {
  public MainImpl(CtorArgs c, InitArgs i);
  public X foo() { ... }
  public Y bar() { ... }
}

This extracts the main behaviour of the class into the MainImpl.

Answer 2

The most effective and helpful way to prevent clients from "misusing" an object is by making it impossible.

The simplest solution is to merge Initialize with the constructor. That way, the object will never be available for the client in the uninitialized state so the error is not possible. In case you cannot do the initialization in the constructor itself, you can create a factory method. For example, if your class requires certain events to be registered, you could require the event listener as a parameter in the constructor or factory method signature.

If you need to be able to access the uninitialized object before initialization, then you could have the two states implemented as separate classes, so you start out with an UnitializedFooManager instance, which have an Initialize(...) method which returns InitializedFooManager. The methods which can only be called in the initialized state only exist on InitializedFooManager. This approach can be extended to multiple states if you need to.

Compared to runtime exceptions, it is more work to represent states as distinct classes, but it also gives you compile-time guarantees that you are not calling methods which are invalid for the object state, and it documents the state transitions more clearly in code.

But more generally, the ideal solution is to design your classes and methods without constraints such as requiring you to call methods at certain times and in a certain order. It might not always be possible, but it can be avoided in many cases by using appropriate pattern.

If you have complex temporal coupling (a number of methods must be called in a certain order), one solution could be to use inversion of control, so you create a class which calls the methods in the appropriate order, but uses template methods or events to allow the client to perform custom operations at appropriate stages in the flow. That way, the responsibility for performing operations in the right order is pushed from the client to the class itself.

A simple example: You have a File-object which allows you to read from a file. However, the client needs to call Open before the ReadLine method can be called, and needs to remember to always call Close (even if an exception occurs) after which the ReadLine method must not be called anymore. This is temporal coupling. It can be avoided by having a single method which takes a callback or delegate as an argument. The method manages opening the file, calling the callback and then closing the file. It can pass a distinct interface with a Read method to the callback. That way, it is not possible for the client to forget to call methods in the correct order.

Interface with temporal coupling:

class File {
     /// Must be called on a closed file.
     /// Remember to always call Close() when you are finished
     public void Open();

     /// must be called on on open file
     public string ReadLine();

     /// must be called on an open file
     public void Close();
}

Without temporal coupling:

class File {
    /// Opens the file, executes the callback, and closes the file again.
    public void Consume(Action<IOpenFile> callback);
}

interface IOpenFile {
    string ReadLine();
}

A more heavyweight solution would be to define File as an abstract class which requires you to implement a (protected) method which will be executed on the open file. This is called a template method pattern.

abstract class File {
    /// Opens the file, executes ConsumeOpenFile(), and closes the file again.
    public void Consume();

    /// override this
    abstract protected ConsumeOpenFile();

    /// call this from your ConsumeOpenFile() implementation
    protected string ReadLine();
}

The advantage is the same: The client does not have to remember to call methods in a certain order. It is simply not possible to call methods in the wrong order.

Answer 3

I would normally just check for intiialisation and throw (say) an IllegalStateException if you try and use it whilst not initialised.

However, if you want to be compile-time safe (and that is laudable and preferable), why not treat the initialisation as a factory method returning a constructed and initialised object e.g.

ComponentBuilder builder = new ComponentBuilder();
Component forClients = builder.initialise(); // the 'Component' is what you give your clients

and so you control the objects creation and lifecycle, and your clients get the Component as a result of your initialisation. It's effectively a lazy instantiation.

Answer 4

I'm going to break a little bit from the other answers and disagree: it's impossible to answer this without knowing what language you're working in. Whether or not this is a worthwhile plan, and the right sort of "warning" to give your users depends entirely on the mechanisms that your language provides and the conventions other developers of that language tend to follow.

If you have a FooManager in Haskell, it would be criminal to allow your users to build one that isn't capable of managing Foos, because the type system makes it so easy and those are the conventions that Haskell developers expect.

On the other hand, if you're writing C, your coworkers would be entirely within their rights to take you out back and shoot you for defining separate struct FooManager and struct UninitializedFooManager types that support different operations, because it would lead to unnecessarily complex code for very little benefit.

If you're writing Python, there's no mechanism in the language to let you do this.

You are probably not writing Haskell, Python, or C, but they're illustrative examples in terms of how much/how little work the type system is expected and able to do to do.

Follow the reasonable expectations of a developer in your language, and resist the urge to over-engineer a solution that does not have a natural, idiomatic implementation (unless the mistake is so easy to make and so hard to catch that it's worth going to extreme lengths to make it impossible). If you don't have enough experience in your language to judge what's reasonable, follow the advice of someone who knows it better than you.

Answer 5

As you seem to not want to ship the code check to the customer (but seem fine to do it for the programmer) you could use assert functions, if they are available in your programming language.

That way you have the checks in the development environment (and any tests a fellow dev would call WILL fail predictably), but you will not ship the code to the customer as asserts (at least in Java) are only selectively compiled.

So, a Java class using this would look like this:

/** For some reason, you have to call init and connect before the manager works. */
public class FooManager {
   private int state = 0;

   public FooManager () {
   }

   public synchronized void init() {
      assert state==0 : "you called init twice.";
      // do something
      state = 1;
   }

   public synchronized void connect() {
      assert state==1 : "you called connect before init, or connect twice.";
      // do something
      state = 2;
   }

   public void someWork() {
      assert state==2 : "You did not properly init FooManager. You need to call init and connect first.";
      // do the actual work.
   }
}

Assertions are a great tool to check for runtime state of your program that you require, but don't actually expect to anyone ever doing wrong in a real environment.

Also they are quite slim and don't take up huge portions of if() throw... syntax, they don't need to be caught, etc.

Answer 6

Looking at this problem more generally than the current answers do, which have mostly focussed on initialization. Consider an object that will have two methods, a() and b(). The requirement is that a() is always called before b(). You can create a compile-time check that this happens by returning a new object from a() and moving b() to the new object rather than the original one. Example implementation:

class SomeClass
{
   private int valueRequiredByMethodB;

   public IReadyForB a (int v) { valueRequiredByMethodB = v; return new ReadyForB(this); }

   public interface IReadyForB { public void b (); }

   private class ReadyForB : IReadyForB
   {
      SomeClass owner;
      private ReadyForB (SomeClass owner) { this.owner = owner; }
      public void b () { Console.WriteLine (owner.valueRequiredByMethodB); }
   }
}

Now, it is impossible to call b() without first calling a(), as it is implemented in an interface that is hidden until a() is called. Admittedly, this is quite a lot of effort, so I wouldn't usually use this approach, but there are situations where it can be beneficial (especially if your class is going to be reused in a lot of situations by programmers who might not be familiar with its implementation, or in code where reliability is critical). Also note that this is a generalisation of the builder pattern, as suggested by many of the existing answers. It works pretty-much the same way, the only real difference is where the data is stored (in the original object rather than the one returned) and when it's intended to be used (at any time versus only during initialisation).

Answer 7

When I implement a base class that requires extra initialisation information or links to other objects before it becomes useful, I tend to make that base class abstract, then define several abstract methods in that class that are used in the flow of the base class (like abstract Collection<Information> get_extra_information(args); and abstract Collection<OtherObjects> get_other_objects(args); which by contract of inheritance must be implemented by the concrete class, forcing the user of that base class to supply all the things required by the base class.

Thus when I implement the base class, I immediately and explicitly know what I have to write to make the base class behave correctly, since I just need to implement the abstract methods and that's it.

EDIT: to clarify, this is almost the same as providing arguments to the constructor of the base class, but the abstract method implementations allow the implementation to process the arguments passed to the abstract method calls. Or even in the case of no arguments being used, it can still be useful if the return value of the abstract method depended on a state that you can define in the method body, which isn't possible when passing the variable as an argument to the constructor. Granted you can still pass an argument that will have dynamic behaviour based on the same principle if you'd rather use composition over inheritance.

Answer 8

The answer is: Yes, no, and sometimes. :-)

Some of the problems you describe are easily solved, at least in many cases.

Compile-time checking is certainly preferable to run-time checking, which is preferable to no checking at all.

RE run-time:

It is at least conceptually simple to force functions to be called in a certain order, etc, with run-time checks. Just put in flags that say which function has been run, and let each function begin with something like "if not pre-condition-1 or not pre-condition-2 then throw exception". If the checks get complex, you could push them into a private function.

You can make some things happen automagically. To take a simple example, programmers often talk about "lazy population" of an object. When the instance is created, you set an is-populated flag to false, or some key object reference to null. Then when a function is called that needs the relevant data, it checks the flag. If it's false it populates the data and sets the flag true. If it's true it just goes on assuming the data is there. You can do the same thing with other preconditions. Set a flag in the constructor or as a default value. Then when you reach a point where some pre-condition function should have been called, if it hasn't been called yet, call it. Of course this only works if you have the data to call it at that point, but in many cases you can include any required data in the function parameters of the "outer" call.

RE compile-time:

As others have said, if you need to initialize before an object can be used, that put the initialization in the constructor. This is pretty typical advice for OOP. If at all possible, you should make the constructor create a valid, usable instance of the object.

I see some discussion about What to do if you need to pass around references to the object before it's initialized. I can't think of a real example of that off the top of my head, but I suppose it could happen. Solutions have been suggested, but the solutions I've seen here and any I can think of are messy and complicate the code. At some point you have to ask, Is it worth making ugly, hard-to-understand code so we can have compile-time checking rather than run-time checking? Or am I creating a lot of work for myself and others for a goal that is nice but not required?

If two functions should always be run together, the simple solution is to make them one function. Like if every time you add an advertisement to a page you should also add a metrics handler for it, then put the code to add the metrics handler inside the "add advertisement" function.

Some things would be almost impossible to check at compile time. Like a requirement that a certain function cannot be called more than once. (I've written many functions like that. I just recently wrote a function that looks for files in a magic directory, processes any it finds, and then deletes them. Of course once it's deleted the file it's not possible to re-run. Etc.) I don't know any language that has a feature that allows you to prevent a function from being called twice on the same instance at compile time. I don't know how the compiler could definitively figure out that that was happening. All you can do is put in run time checks. That particularly run time check is obvious, isn't it? "if already-been-here = true then throw exception else set already-been-here = true".

RE impossible to enforce

It's not uncommon for a class to require some sort of clean-up when you're done: closing files or connections, freeing memory, writing final results to the DB, etc. There's no easy way to force that to happen at either compile time or run time. I think most OOP languages have some sort of provision for a "finalizer" function that gets called when an instance is garbage collected, but I think most also say that they cannot guarantee this will ever be run. OOP languages often include a "dispose" function with some sort of provision to set a scope on the use of an instance, a "using" or "with" clause, and when the program exits that scope the dispose is run. But this requires the programmer to use the appropriate scope specifier. It doesn't FORCE the programmer to do it right, it just makes it easier for him to do it right.

In cases where it is impossible to force the programmer to use the class correctly, I try to make it so the program blows up if he does it wrong, rather than give incorrect results. Simple example: I've seen lots of programs that initialize a bunch of data to dummy values, so that the user will never get a null-pointer exception even if he fails to call the functions to populate the data correctly. And I always wonder why. You're going out of your way to make bugs harder to find. If, when a programmer failed to call the "load data" function and then blithely tried to use the data, he got a null pointer exception, the exception would quickly show him where the problem was. But if you hide it by making the data blank or zero in such cases, the program may run to completion but produce inaccurate results. In some cases the programmer may not even realize there was a problem. If he does notice, he may have to follow a long trail to find where it got wrong. Better to fail early than try to bravely carry on.

In general, sure, it's always good when you can make incorrect use of an object simply impossible. It's good if functions are structured in such a way that there's simply no way for the programmer to make invalid calls, or if invalid calls generate compile-time errors.

But there's also some point at which you have to say, The programmer should read the documentation on the function.

Answer 9

Yes, your instincts are spot-on. Many years ago I wrote articles in magazines (kind of like this place, but on dead-tree and put out once per month) discussing Debugging, Abugging, and Antibugging.

If you can get the compiler to catch the misuse, that is the best way. What language features are available depend on the language, and you did not specify. Specific techniques would be detailed for individual questions on this site, anyway.

But having a test to detect the usage at compile time instead of run-time is really the same kind of test: you still need to know to call the proper functions first and use them in the right way.

Designing the component to avoid that even being an issue in the first place is much more subtle, and I like the Buffer is like the Element example. Part 4 (published twenty years ago! Wow!) contains an example with discussion that's pretty similar to your case: This class must be used carefully, as buffer() may not be called after you start using read(). Rather, it must only be used once, immediately after construction. Having the class manage the buffer automatically and invisibly to the caller would avoid the problem conceptually.

You ask, if tests can be done at run-time to ensure proper usage, should you do that?

I would say yes. It will save your team a great deal of debugging work some day, when code is maintained and the specific usage gets perturbed. The testing can be set up as a formal set of constraints and invariants that can be tested. That doesn't mean the overhead of extra space to track the state and work to do the checking is always left in for every call. It's in the class so it could be called, and the code documents the actual constraints and assumptions.

Perhaps the checking is only done in a debug build.

Perhaps the check is complex (think of heapcheck, for example), but it's present and can be done once in a while, or calls added here and there when the code is being worked on and some problem surfaced, or to do specific testing to make sure there isn't any such problem in a unit-testing or integration testing program that links to the same class.

You may decide that the overhead is trivial after all. If the class does file I/O, and extra state byte and a test for such is nothing. Common iostream classes check for the stream being in the bad state.

Your instincts are good. Keep it up!

Answer 10

The principle of Information Hiding (Encapsulation) is that entities outside the class should not know more than what's required to use this class properly.

Your case seems like you are trying to get an object of a class to run properly without telling the outside users enough about the class. You have hidden more information than you should have.

• Either explicitly ask for that required information in the constructor;
• Or keep the constructors intact and modify the methods so that in order to call the methods, you have to provide the missing data.

---

Words Of Wisdom:

_{* Either way you have to redesign class and/or constructor and/or methods.}

_{* By not designing properly and starving the class from correct information you are risking your class to break at not one but potentially multiple places.}

_{* If you have written the exceptions and error messages poorly, you class will be giving away even more about itself.}

_{* Finally, if the outsider be supposed to know that it has to initialize a, b and c and then call d(), e(), f(int) then you have a leak in your abstraction.}

Answer 11

Since you've specified that you're using C# a viable solution to your situation is to leverage the Roslyn Code Analyzers. This will allow you to catch violations immediately and even allow you to suggest code fixes.

One way to implement it would be to decorate the temporally coupled methods with an attribute that specifies the order that the methods need to be called¹. When the analyzer finds these attributes in a class it validates that the methods are called in order. This would make your class look something like the following:

public abstract class Foo
{
   [TemporallyCoupled(1)]
   public abstract void Init();

   [TemporallyCoupled(2)]
   public abstract void DoBar();

   [TemporallyCoupled(3)]
   public abstract void Close();
}

---

1: I've never written a Roslyn Code Analyzer so this might not be the best implementation. The idea to use Roslyn Code Analyzer to verify your API is being used correctly is 100% sound though.

Answer 12

The way I've solved this problem before was with a private constructor and a static MakeFooManager() method within the class. Example:

public class FooManager
{
     public string Foo { get; private set; }
     public string Bar { get; private set; }

     private FooManager(string foo, string bar)
     {
         Foo = foo;
         Bar = bar;
     }

     public static FooManager MakeFooManager(string foo, string bar)
     {
         // Can do other checks here too.
         if(foo == null || bar == null)
         {
             return null;
         }
         else
         {
             return new FooManager(foo, bar);
         }
     }
}

Since a constructor is implemented, there is no way for anyone to create an instance of FooManager without going through MakeFooManager().

Answer 13

There are several approaches:

1. Make the class easy to use right and hard to use wrong. Some of the other answers focus exclusively on this point, so I'll simply mention RAII and the builder pattern.

2. Be mindful of how to use comments. If the class is self explaining, don't write comments.* That way people are more likely to read your comments when the class actually needs them. And if you have one of these rare cases where a class is difficult to use correctly and needs comments, include examples.

3. Use asserts in your debug builds.

4. Throw exceptions if the usage of the class is invalid.

*These are the kinds of comments you should not write:

// gets Foo
GetFoo();