ParserError: Expected ',' but got identifier

Question

pragma solidity >=0.4.0 <0.7.0;

contract NewHello{

    struct kullanicilar{
        string name;
        string surname;
        uint age;
    }

    event infoChanged(
        string name,
        string surname,
        uint age
    );

    uint newUser;
    mapping(address => string) public users; // contains a name for every address
    //[] public userAddress ;

    function updateUser(string memory name , string memory surname , unit age) public returns(uint userID){
        userID = newUser++;
        users[userID] = kullanicilar(string name, string surname, age);
        emit infoChanged(name,surname,age);
    }
    function getUser(address a) public view returns(string memory) {
        return users[a]; // return address name
    }
}

Hello ! I want to tell you what I want. I built a sturct name is kullanıcılar(means users in Turkish) and I built a event for if I changed my informations about my users I want to see my event. finally I made getUser func which is return to me the name of this user.

but I have got a error at updateUser function which is

ParserError: Expected ',' but got identifier users[userID] = kullanicilar(string name, string surname, age); ^--^

I checked my codes and I can't find my error anyone can help me for this :) thanks for read :)

Answer 1

You should not specify the types of parameters when you pass them in a function-call:

users[userID] = kullanicilar(string name, string surname, age);

Change it to:

users[userID] = kullanicilar(name, surname, age);

Answer 2

heyyy I found my answer guys :) thanks for the contribution

the answer is just chance the mapping and forget the getUser function.

code is :

pragma solidity >=0.4.0 <0.7.0;

contract NewHello{

struct user{
    string name;
    string surname;
    uint age;
}

event infoChanged(
    string name,
    string surname,
    uint age
);

uint newUser;
mapping(uint => user) public users; // contains a name for every address
//[] public userAddress ;

function updateUser(string memory name , string memory surname , uint age) public returns(uint userID){
    userID = newUser++;
    users[userID] = user(name, surname, age);

    emit infoChanged(name,surname,age);
}
//function getUser(uint a) public view returns(string memory) {
//    return users[a]; // return address name
//}

}

Answer 3

As mentioned by @goodvibration, you do not need to specify the types of the struct members when you declare a new struct "instance" ( new struct instance = new variable of type struct).

Because the type is already specified in your initial struct definition

This is why the compiler returns an error message : Basically, the Solidity compiler expects a string value (either as a variable, or directly hardcoded string into your argument, like "John Doe"). So it consider the first string keyword as the value.

Expected , but got identifier :
users[userID] = kullanicilar(string name, string surname, age);
                                    ^--^

---

How to declare a Struct in Solidity ?

Let's go back to your example :

struct User {
    string name;
    string surname;
    uint age;
}

// Rename `newUser` by `total_users`
// uint newUser;
uint total_users;

mapping(uint => User) public users;

Before you might consider the following :

1. According to the Solidity documentation, the naming convention for struct is to use CapWords.. So it would look nicer (for reading purpose) to use User, instead of user.

2. Since you are using newUser++ (to add a new user I suppose), you should rename your function setUser and not updateUser.

3. You can't loop through a mapping. So you should implement 1) an array on top, and 2) a variable total_users as an index that increments when you add a new user. Like the Campaign contract example from the Solidity doc, look at the newCampaign function.

Back to the subject !

You have three ways in Solidity to implement a custom struct into a variable :

---

The conventional way

function setUserMethod1(string memory _name , string memory _surname , uint _age) public returns(uint userID){
    userID = total_users++;
    User memory new_user;
    new_user.name = _name;
    new_user.surname = _surname;
    new_user.age = _age;

    users[userID] = new_user;

    emit infoChanged(_name,_surname,_age);
}

Firstly, you create an empty User struct that you store in memory. Secondly, You assign your parameters to the struct member via 3 separate instructions. Finally, you assign your new_user (stored in memory) as the value for the key you specified in your mapping.

Transaction cost : 93975 Execution cost : 71039

---

The readable way

function setUserMethod2(string memory _name , string memory _surname , uint _age) public returns(uint userID) {
    userID = total_users++;
    users[userID] = User({
        name: _name,
        surname: _surname,
        age: _age
    });
    emit infoChanged(_name,_surname,_age);
}

I personally think this is the most readable (Javascript like) and efficient way (2 lines of code removed).

You simply 1) point to a key in the mapping (Using your newly generated userID) and 2) Creating the struct directly in the value assignment.

The parentheses () of User instantiate the Struct (like you instantiate a Javascript object) and the {} contain the members of your struct.

You then specify you function parameters for each specific member (Note that you use colons : for Struct Member -> Value assignment, like you would do for an object in Javascript

Transaction cost : 93786 Execution cost : 70914

---

The shorter way

function setUserMethod3(string memory _name , string memory _surname , uint _age) public returns(uint userID) {
    userID = total_users++;
    users[userID] = User(_name, _surname, _age);
    emit infoChanged(_name,_surname,_age);
}

This is the shorter way to initialise your custom struct type variable. With a syntax similar to a constructor function in Javascript..

Since the struct members are not mentioned, it's a bit harder to guess (might need to scroll back up to the struct definition to understand which value is assigned to which member.

Transaction cost : 93806 Execution cost : 70870

---

Hope it helps :)