Closed spalladino closed 6 years ago
The problem with this approach is that the initialization for one contract could be run twice, which doesn't happen with Migratable
. If we're willing to risk that then I think I'd prefer the simpler approach of adding an initializing
variable and changing isInitializer
to check require(initializing || !initialized)
. I think this has the same effect as this PR and the Initializable
API remains the same.
It could, but only if the developer explicitly invokes onInitialization
again. This a similar scenario to the FinalizableCrowdsale (from OZ) finalize
public method (that checks the finalized flag) vs finalization
internal method, without restrictions, that has the actual logic.
The difference with finalization
is that the recommended usage is through super.finalization
, so that Solidity's linearization of parents takes care of running everything only once. In this case we're recommending to manually call each parent's onInitialize
.
I implemented my suggestion in https://github.com/zeppelinos/labs/pull/112. Please let me know what you think.
The current implementation of initializable fails when used with multiple inheritance, where each parent defines its own initializer, since all parents depend on a single
initialized
flag. Thus, when the first parent is initialized, the second fails to do so.A way to work around this is for every class to define separate
initialize
andonInitialize
methods, whereinitialize
is public, checks and sets the initialized flag, but delegates all actual logic toonInitialize
, which is internal. Seecontracts/test/Inheritance.sol
for an example.