condition check of struct mapping in solidity?

Question

struct User {
       string username;
       string password;
       address userid;
        bool isAdmin;
}

mapping (uint => User) users;

function checkIfUserExists(address userid) public returns(bool) {
    for (uint o = 0; o < totalUsers; o++) {
        if (users[o].userid == userid) {
            return true;
        } else{
            return false;
        }
}

This Always returns false?? even if userid exists in the mapping..

Answer 1

following some simple style guide will make your question more easier to understand and solve!, as goodvibration in comments said, that is not the best way to store a user and check if he/she exists

struct User {
    string username;
    string password;
    address userid;
    bool isAdmin;
}

mapping (uint => User) users;

function checkIfUserExists(address _userid) public returns(bool) {

    for (uint index = 0; index < totalNumberOfUsers; index++) {
        if (users[index].userid == _userid) {
            return true;
        }
    }
    return false;
}

Answer 2

Just chiming in for the benefit of others who come across this.

@Majd is right about the logic error. Even so, there are additional errors in this.

The for loop is an anti-pattern. Its mere existence is not a problem, but the fact that there could be no upper limit to the number of users, therefore the number of iterations, therefore the gas cost of this is a well-known anti-pattern. More here: https://blog.b9lab.com/getting-loopy-with-solidity-1d51794622ad

It doesn't make sense to store a username, password and userid on the chain. This information would be visible to everyone, and it adds cost. It's not necessary to do so because Ethereum transaction signing ensures that all everyone who uses the contract is reliably authenticated.

Instead, map their addresses to user profiles.

If there is a requirement to collect additional information or grant permission to join the system, then a bit for isUser, is appropriate. In most cases that would be unwanted de-anonymization and unnecessarily intrusive. It may be perfectly fine to assume that all users exist. That would reduce the struct to a single property - some users are admins.

pragma solidity 0.5.1;

contract UserExample {

    struct UserStruct {
        // string username;   // use the address
        // string password;   // don't store this on chain
        // address userid;
        bool isUser;          // only needed if there is user initialization
        bool isAdmin;
    }

    mapping(address => UserStruct) public userStructs;

    function isUser(address user) public view returns(bool) {
        return userStructs[user].isUser;
    }
}

See also: Are there well-solved and simple storage patterns for Solidity?

Hope it helps.

Answer 3

The following code works as you need:

pragma solidity 0.5.0;

contract UserManager {
   struct User {
      string username;
      string password;
      address userid;
      bool isAdmin;
   }

   mapping (uint => User) users;
   uint totalUsers=0;

   function checkIfUserExists(address userid) public view returns(bool) {
       for (uint o = 0; o < totalUsers; o++) {
           if (users[o].userid == userid) {
               return true;
           } else{
               return false;
           }
       }
   }

   function addUser(address userid) public {
       User memory user = User("m", "n", userid, true);

       users[totalUsers] = user;
       totalUsers++;
   }

}

I think your implementation was like below which returned false always.

pragma solidity 0.5.0;

contract UserManager {
   struct User {
      string username;
      string password;
      address userid;
      bool isAdmin;
   }

   mapping (uint => User) users;
   uint totalUsers=0;

   function checkIfUserExists(address userid) public view returns(bool) {
       for (uint o = 0; o < totalUsers; o++) {
           if (users[o].userid == userid) {
               return true;
           } else{
               return false;
           }
       }
   }

   function addUser(address userid) public {
       User memory user = User("m", "n", userid, true);
       totalUsers++;
       users[totalUsers] = user;

   }

}

As a result, in the array in index 0, there was no entry which returned false always.

Hope this will help you.