0

How do I make it so this only adds an identifier to the dataArray if it doesn't exist already?

pragma experimental ABIEncoderV2;

contract structWithMapping{

    struct Data{
        string[] user;
        string[] catagory;
        string[]  data;
    }

    mapping(string => Data) mappedData;
    string[] public dataArray;

    function setUserData(string _user, string _catagory, string _data)public{
        var addData = mappedData[_user];

        addData.user.push(_user);
        addData.catagory.push(_catagory);
        addData.data.push(_data);

        dataArray.push(_user) -1;
    }

    function getUsers() view public returns(string[]){
        return dataArray;
    }

    function getUserData(string _user) view public returns(string[] memory, string[] memory, string[] memory){
        return(mappedData[_user].user, mappedData[_user].catagory, mappedData[_user].data);
    }
}
  • Gee. Less words and more code. Please. A short piece of code equals a thousand words. – goodvibration Jun 9 at 19:35
  • Fixed thanks for your time – WesternWarrior Jun 9 at 20:37
0

In order to avoid For loops, You can add another mapping to check if a user exists or not. You can add mapping(string => bool) userExists; so your whole code will look like this:

pragma experimental ABIEncoderV2;


contract structWithMapping{

    struct Data{
        string[] user;
        string[] catagory;
        string[] data;
    }


    mapping(string => Data) mappedData;
    mapping(string => bool) userExists;

    string[] public dataArray;

    function setUserData(string _user, string _catagory, string _data)public{
        var addData = mappedData[_user];


        addData.user.push(_user);
        addData.catagory.push(_catagory);
        addData.data.push(_data);

        require(!userExists[_user]);
        dataArray.push(_user) -1;
        //dont forget to set the mapping value to true for userExists
        userExists[_user] = true;
    }

    function getUsers() view public returns(string[]){
        return dataArray;
    }

    function getUserData(string _user) view public returns(string[] memory, string[] memory, string[] memory){
        return(mappedData[_user].user, mappedData[_user].catagory, mappedData[_user].data);
    }
}
| improve this answer | |
  • When I try to push a new user with the same name I'm getting error transact to structWithMapping.setUserData errored: VM error: revert. revert The transaction has been reverted to the initial state. Note: The called function should be payable if you send value and the value you send should be less than your current balance. Debug the transaction to get more information. I've tried changing the array to a string as the single user can have multiple data entries but its still giving me the same problem but theres nothing payable so I don't understand why – WesternWarrior Jun 11 at 16:10
  • This is another problem. If you don't find your answer on google, Please post another question and provide the code so that it's easier to understand. Thanks. – Masoud jt Jun 12 at 2:54
0

Reading your code I think I understood what you want to achieve, but I suggest to completely change the approach. Please note that using a smart contract costs money, so as a developer you should aim to create business logic that is both clean - so it cannot be misinterpreted or misleading - and light - so every transaction costs as few as possible.

To achieve that you need also to deep dive into the EVM and Solidity or Vyper.

That said, I think your entire smart contract should be transformed to this:

contract structWithMapping {

    struct User {
        string  username;
        string  category;
        string  data;
        bool    initialized;
    }
    mapping(string => User) public users;

    function setUserData(string memory _username, string memory _category, string memory _data) public{
        require(!users[_username].initialized);
        users[_username].username = _username;
        users[_username].category = _category;
        users[_username].data = _data;
        users[_username].initialized = true;
    }

}

Note also that Solidity adds getter functions at compile time for every public variable, so in this case a users function will be created. You can use that function passing the username as parameter to retrieve user data in the exact same way you would have called getUserData function.

I created a gist so you can directly experiment through the Remix IDE via this link.

| improve this answer | |
  • Thank you for your time and response, I understand that the getUsers() function is redundant but I don't see a way to retrieve the mapped data when I make the data and category into arrays I'm assuming it has to do with the call data but I guess I don't understand how to reference it properly as it requests a Hexadecimal value that I am unsure where the change from the mapping in this case users(username) translates to – WesternWarrior Jun 10 at 13:53
  • You cannot retrieve all users from this contract with a single call. If you really want to do that, you need to replace the mapping with an array of User and you need to use a second variable to store true/false for initialization, as shown in the response by Masoud jt. But please, remember that Ethereum programming is a complete new way of thinking applications. You should not store contract's state something that is not useful to the contract itself. It's not a database, it's business logic. If there's no logic in your contract, it become simply a really slow and expensive storage system. – Giuseppe Bertone Jun 11 at 16:20
  • To elaborate more, my example was made for you to understand how to check if an element exists, and to clean the majority of the code. If that was a real world smart contract written by me, it will be more like this: "function setUserData(string memory _username, string memory _category, string memory _data) public{ } }". In fact simply a void function just to set the type of the parameter sent by the client. The real data is already present in each transaction the client send. See something like this for more details: medium.com/@childsmaidment/… – Giuseppe Bertone Jun 11 at 16:21
  • The above are best practices and knowledge you should aim to obtain as soon as possible, to avoid programming Smart Contracts as they were common applications. – Giuseppe Bertone Jun 11 at 16:25
  • Thank you for the continued clarifications but I am trying to retrieve a single users mapped data from the getUserData not the entire list of users. I do understand that using the contract as a database slows it down, but other than storing directly on the chain and then querying the entire chain I don't know of other ways to keep the user list. – WesternWarrior Jun 11 at 18:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.