compare strings in solidity

Question

I wrote a function that takes two strings as arguments and compares them and returns bool value. Here is the code.

function compareStringsbyBytes(string s1, string s2) public pure returns(bool)
{
bytes memory s1bytes = bytes(s1);
bytes memory s2bytes = bytes(s2);
if(s1bytes.length!=s2bytes.length) {
    return false;
}
else{
    for(uint i = 0;i<s1bytes.length;i++)
    {
        if(s1bytes[i] != s2bytes[i])
         return false;
    }
    return true;
}
}

The contract deployment failed in this case with transaction out of gas error. Can someone help me understand the problem with this code?

Thanks!!

Answer 1

@henkiedoodle's comment is correct, it would be much simpler and easier to compare hashes of the strings.

You are unable to directly compare strings, (ie return(s1 == s1) will give the error TypeError: Operator == not compatible with types string memory and string memory), but if the strings are identical then so will their hashes be.

The full function could then be simplified to

function compareStringsbyBytes(string s1, string s2) public pure returns(bool){
    return keccak256(s1) == keccak256(s2);
}

However, in future if you receive the out of gas error, you may also want to try increasing the gas limit of your transaction.

Update: the above answer (and the question) were both circa Solidity 0.4.X. For 0.5.X, the following would be a valid compare function

function compareStringsbyBytes(string memory s1, string memory s2) public pure returns(bool){
    return keccak256(abi.encodePacked(s1)) == keccak256(abi.encodePacked(s2));
}

Answer 2

When the contract deployment fails, you should be looking for other issues:

1. The contract does not compile under your version of solc (depends on tools used), meaning you are trying to deploy an empty bytecode.
2. Something in the constructor does not work, e.g. a require() condition is not met.
3. The deployment cost is beyond your unlocked account's balance or it is over the amount of gas supplied (you may have to increase if the estimate is incorrect), or it is over the chain's block gasLimit.

It's unlikely the failure to deploy the contract has anything to do with the design of this function since it is not involved in deployment itself. As an aside, I would upvote the hash comparison idea on grounds of readability and simplicity.

Hope it helps.