poanetwork / token-wizard

(Discontinued) TokenWizard is an DApp to create and manage crowdsale and token contracts using a simple UI
MIT License
384 stars 215 forks source link

(Suggestion): Replace callbacks with promises #318

Closed fvictorio closed 6 years ago

fvictorio commented 6 years ago

This will probably be a big refactor, but I think the impact in maintainability is huge.

Code like this:

setLastCrowdsaleRecursive(0, web3, contracts.pricingStrategy.abi, contracts.pricingStrategy.addr, contracts.crowdsale.addr.slice(-1)[0], 42982, (err) => {
  if (err) return this.hideLoader();
  setReservedTokensListMultiple(web3, contracts.token.abi, contracts.token.addr, this.state.token, (err) => {
    if (err) return this.hideLoader();
    updateJoinedCrowdsalesRecursive(0, web3, contracts.crowdsale.abi, contracts.crowdsale.addr, (err) => {
      if (err) return this.hideLoader();
      setMintAgentRecursive(0, web3, contracts.token.abi, contracts.token.addr, contracts.crowdsale.addr, 68425, (err) => {
        if (err) return this.hideLoader();
        setMintAgentRecursive(0, web3, contracts.token.abi, contracts.token.addr, contracts.finalizeAgent.addr, 68425, (err) => {
          if (err) return this.hideLoader();
          addWhiteListRecursive(0, web3, this.state.crowdsale, this.state.token, contracts.crowdsale.abi, contracts.crowdsale.addr, (err) => {
            if (err) return this.hideLoader();
            setFinalizeAgentRecursive(0, web3, contracts.crowdsale.abi, contracts.crowdsale.addr, contracts.finalizeAgent.addr, 68622, (err) => {
              if (err) return this.hideLoader();
              setReleaseAgentRecursive(0, web3, contracts.token.abi, contracts.token.addr, contracts.finalizeAgent.addr, 65905, (err) => {
                transferOwnership(web3, this.state.contracts.token.abi, contracts.token.addr, this.state.crowdsale[0].walletAddress, 46699, (err) => {
                  if (err) return this.hideLoader();
                  this.hideLoader();
                });
              });
            });
          });
        });
      });
    });
  });
});

would be replaced by:

setLastCrowdsaleRecursive(0, web3, contracts.pricingStrategy.abi, contracts.pricingStrategy.addr, contracts.crowdsale.addr.slice(-1)[0], 42982)
  .then(() => setReservedTokensListMultiple(web3, contracts.token.abi, contracts.token.addr, this.state.token))
  .then(() => updateJoinedCrowdsalesRecursive(0, web3, contracts.crowdsale.abi, contracts.crowdsale.addr))
  .then(() => setMintAgentRecursive(0, web3, contracts.token.abi, contracts.token.addr, contracts.crowdsale.addr, 68425))
  .then(() => setMintAgentRecursive(0, web3, contracts.token.abi, contracts.token.addr, contracts.finalizeAgent.addr, 68425))
  .then(() => addWhiteListRecursive(0, web3, this.state.crowdsale, this.state.token, contracts.crowdsale.abi, contracts.crowdsale.addr))
  .then(() => setFinalizeAgentRecursive(0, web3, contracts.crowdsale.abi, contracts.crowdsale.addr, contracts.finalizeAgent.addr, 68622))
  .then(() => setReleaseAgentRecursive(0, web3, contracts.token.abi, contracts.token.addr, contracts.finalizeAgent.addr, 65905))
  .then(() => transferOwnership(web3, this.state.contracts.token.abi, contracts.token.addr, this.state.crowdsale[0].walletAddress, 46699))
  .then(() => this.hideLoader())
  .catch(() => this.hideLoader())
igorbarinov commented 6 years ago

@fvictorio why not async/await tho?

fvictorio commented 6 years ago

It's another alternative, but it's only supported in recent node versions, so you need to transpile the code (with babel for example) or disallow using older versions of node.

Besides that, and this is just a personal opinion, async/await is pretty new and I'm not sure if best practices around it are already settled, so it might be a little riskier.

Anyway, I'm ok with using it if you prefer it.

igorbarinov commented 6 years ago

I'm fine with promises. @vbaranov what do you think?

vbaranov commented 6 years ago

Totally agree with promises