Solidity – Troubleshooting Multiple If Statements Evaluation Failure

contract-developmentinheritanceremixsolidity

I'm trying to evaluate more than one if statements but after a certain number of if statements remix throws an error: error: Failed to decode output: TypeError: Cannot read property 'length' of undefined. From the error it seems that length is undefined but the same code works fine if I reduce the number if if statements. For the current case, if I test 3 ifs, then its fine but as soon as I add one more if, remix throws an error.

I have two contracts: one is Parent and other one is Child. The Parent contract is instantiated in Child contract and its functions are inherited.

pragma solidity ^0.4.20;

contract Parent {

    struct Info{
        string fName;
        string lName;
    }

    uint256[] values;

    mapping(address => Info) names;
    mapping (address => uint256[])transactions;
    mapping (address => uint) accountBalance;


    function setName(address addr,string _fName, string _lName){
        names[addr] = Info(_fName, _lName);
    }

    function getFName(address addr) constant returns (string){
        return names[addr].fName;
    }

    function getLName(address addr) constant returns (string){
        return names[addr].lName;
    }

    function storeValue(address addr, uint256 _value) {
        transactions[addr].push(_value);
        accountBalance[addr] += _value;
    }

    function accountTx(address addr) constant returns(uint256[]) {
        return (transactions[addr]);
    }

    function totalBalance(address addr) constant returns(uint){
        return accountBalance[addr];
    }

}

and this is Child contract:

pragma solidity ^0.4.20;

import './Parent1.sol';

contract Child {

  /* instantiating parent contract*/
  Parent p = Parent(0x692a70d2e424a56d2c6c27aa97d1a86395877b3a);

  uint256 public totalSupply;
  mapping (address => uint256) balances;

  function getName(address addr)constant returns(string, string){
      return (p.getFName(addr), p.getLName(addr));
  }

  function addBalance(address addr){
      if (uint(keccak256(p.getFName(addr))) == uint(keccak256("a")) && uint(keccak256(p.getLName(addr))) == uint(keccak256("b"))){
          balances[addr] += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * 2;
          totalSupply += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * 2;

      }

      if (uint(keccak256(p.getFName(addr))) == uint(keccak256("a")) && uint(keccak256(p.getLName(addr))) == uint(keccak256("c"))){
          balances[addr] += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * 3;
          totalSupply += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * 3;

      }

      if (uint(keccak256(p.getFName(addr))) == uint(keccak256("a")) && uint(keccak256(p.getLName(addr))) == uint(keccak256("d"))){
          balances[addr] += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * 4;
          totalSupply += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * 4;

      }

      if (uint(keccak256(p.getFName(addr))) == uint(keccak256("a")) && uint(keccak256(p.getLName(addr))) == uint(keccak256("e"))){
          balances[addr] += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * 5;
          totalSupply += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * 5;

      }

      else {
          balances[addr] += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * 6;
          totalSupply += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * 6;

      }

  }

  function checkBalance(address addr) constant returns(uint){
      return balances[addr];
  }

}

If you just create two contracts in remix by copy pasting the codes as it is, and then deploy the parent contract, set some names and values, copy the address of deployed contract and replace the address here:

Parent p = Parent(0x692a70d2e424a56d2c6c27aa97d1a86395877b3a);

you will get the error, but if you just remove the one if condition, it will work fine.

The contracts are simplified version of my original contracts and I tried some alternatives but no luck.

Can someone help me out why this strange behaviour is and how can I continue with more than one ifs?

with thanks,

Best Answer

From what you've said, it is when you are testing with certain values that one of your if statements throws an error. Did you check that the transactions mapping is not empty for the values you are testing with? You've asked people to do fairly random testing for you. Without more specific information about the values you have used to produced the error and in which if statement the error occurred it is difficult to understand how you produce the error in the first place.

You could try to reduce some complexity in the code to make it more efficient also help make errors easier to trace.

I'd break your addBalance() function into two parts by creating a getMultiplier() function because the if statements are just trying to identify which multiplier value to use. 

function addBalance(address addr){
    uint multiplier;
    multiplier = getMultiplier(addr);

    // You should check that you have a multiplier value before updating your balance and totalSupply values.
    balances[addr] += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * multiplier;
    totalSupply += (p.accountTx(addr)[p.accountTx(addr).length - 1]) * multiplier;  
}

function getMultiplier(address addr) returns (uint){

  if ( uint(keccak256(p.getFName(addr))) == uint(keccak256("a")) ) { 

    if ( uint(keccak256(p.getLName(addr))) == uint(keccak256("b")) ){
        return 2;
    }

    if ( uint(keccak256(p.getLName(addr))) == uint(keccak256("c")) ){
        return 3;
    }

    if ( uint(keccak256(p.getLName(addr))) == uint(keccak256("d")) ){
        return 4;
    }

    if ( uint(keccak256(p.getLName(addr))) == uint(keccak256("e")) ){
         return 5;
    } else {
        return 6;
    }
  }
}


Each of your if statements started with the same condition.

if( uint(keccak256(p.getFName(addr))) == uint(keccak256("a")) && ....

If this condition fails the first time you shouldn't continue checking every other if statement because you already know they will fail as well. So you move this into a pre-condition before starting to check each of the last name conditions.

Using the getMultiplier() function also makes your code easier to maintain. Instead of the calculation for updating balances and totalSupply being repeated in each if statement it's much better to have this in only one location so you can clearly see when values are being modified. The getMultiplier() will also break out of the function (using return) when the correct value is found whereas your method continues to read every if statement until it reaches the end of the function which is time-consuming.

Instead of calling the getFName() and getLName() functions for every if statement you can hold these values in variables at the start of the function and then just read from the variable instead of multiple function calls to get the same result.

Related Topic