Module Registry removeModule() function #244
Conversation
|
@thegostep why not just skip the whole re-ordering code block if array length is == 1? |
contracts/ModuleRegistry.sol
Outdated
| @@ -20,6 +20,8 @@ contract ModuleRegistry is IModuleRegistry, Pausable, RegistryUpdater, ReclaimTo | |||
| mapping (address => address[]) public reputation; | |||
| // Mapping contain the list of addresses of Module factory for a particular type | |||
| mapping (uint8 => address[]) public moduleList; | |||
| // Mapping to store the index of the moduleFacorty in the moduleList | |||
contracts/ModuleRegistry.sol
Outdated
| require(moduleFactory.getType() != 0, "Factory type should not equal to 0"); | ||
| registry[_moduleFactory] = moduleFactory.getType(); | ||
| moduleList[moduleFactory.getType()].push(_moduleFactory); | ||
| uint8 kind = moduleFactory.getType(); |
There was a problem hiding this comment.
instead of "kind" use "type"
There was a problem hiding this comment.
type is a reserved keyword documentation
There was a problem hiding this comment.
I mean, moduleFactoryType or _type, don't want to be introducing different names.
|
edit: I can add a check |
|
In response to this:
I meant instead of adding an item to index 0, change the logic so if there's only one item you don't re-order but just remove that item. |
|
Looks good to me now. @adamdossa please review. |
Please double check logic for dealing with array index 0. My approach has been to set index 0 of the array to the 0x0 address and prevent from removing, the side effect is that getting the list of module factories will always include the 0x0 address.