Solidity - The Constructor should be Payable if you send value during transfer

Question

I was just wondering and do some search on how token works, and what I get is :

Send ethereum to smart contract => Smart contract have a price for the token, the value, and return back the token to msg.sender as address by dividing amount of value sent and the token price, so I created following contract

pragma solidity ^0.4.0;

contract SimpleToken {
    address client;
    uint256 public totalSupply;
    uint remaining;
    uint price;

uint ethval;

mapping (address => uint) investors;
event testDebug(string _msg,address client,uint256 totalSupply,uint receivedValue);
function SimpleToken(uint _totalSupply) //constructor
{
    client = msg.sender;
    totalSupply = _totalSupply;
    remaining = 0;
}

function setEthValue(uint _value){
    ethval = _value;
}

function sendToken() payable 
{
    testDebug("Before",client,totalSupply,investors[msg.sender]);
    assert(remaining < totalSupply);
    uint noOfTokens = ethval/price;
    assert(noOfTokens < totalSupply-remaining); 
    investors[msg.sender] = noOfTokens;
    remaining = remaining+noOfTokens;

    testDebug("After",client,remaining,noOfTokens);

    client.transfer(noOfTokens);

}

function setTokenPrice(uint _price)
{ 
    price = _price;
}

function getEthVal() constant returns(uint){
    return ethval;
}
}

Then I set the input value as follows :

When I run sendToken function which will be sending token to sender account, why does it return error

The Constructor should be Payable if you send value during transfer

I tried adding payable to the constructor but it gave me no luck either.

What is wrong with my simple contract ?

Answer 1

A few possibilities:

1. price could be 0, resulting in a divide-by-zero.
2. price could be such that noOfTokens is greater than the remaining tokens.
3. If neither of those are stopping things first, client.transfer(noOfTokens) probably is.

client.transfer(noOfTokens) tries to transfer ether to client. The amount of ether attempted in the transfer is noOfTokens wei. I see no reason to expect the contract to have that much ether.

You probably want this (msg.value instead of ethval) and transferring that much to client:

function sendToken() payable 
{
    testDebug("Before",client,totalSupply,investors[msg.sender]);
    assert(remaining < totalSupply);
    uint noOfTokens = msg.value / price;
    assert(noOfTokens < totalSupply-remaining); 
    investors[msg.sender] = noOfTokens;
    remaining = remaining+noOfTokens;

    testDebug("After",client,remaining,noOfTokens);

    client.transfer(msg.value);    
}

Just be sure to actually transfer ether when you call sendToken(). (You can set the amount of ether sent with the transaction in the upper right in Remix with the "value" field.)