MyContract.sol

pragma solidity ^0.4.4;

// This is a simple contract that keeps track of balances and allows withdrawals only after a week from the deposit.
// 1. In your opinion, does withdraw() function work as expected?
// 2. Implement the missing deposit() function that will allow only a single deposit from any address

contract MyContract {
  mapping (address => uint) balances;
  mapping (address => uint) deposits;

  event Transfer(address indexed _from, address indexed _to, uint256 _value);

  function withdraw(uint amount) returns(bool) {
    uint depositTime = deposits[tx.origin];
    uint balance = balances[tx.origin];

    if (now > depositTime + 7 days) {
      return false;
    }
    if (balance <= amount) return false;

    if(msg.sender.call.value(balance)()) {
      balances[msg.sender] -= amount;
    }

    Transfer(this, msg.sender, amount);
    return true;
  }

}

pragma solidity ^0.4.4;

// This is a simple contract that keeps track of balances and allows withdrawals only after a week from the deposit.
// 1. In your opinion, does withdraw() function work as expected?
// 2. Implement the missing deposit() function that will allow only a single deposit from any address

contract MyContract {
mapping (address => uint) balances;
mapping (address => uint) deposits;

event Transfer(address indexed _from, address indexed _to, uint256 _value);

function withdraw(uint amount) returns(bool) {

uint depositTime = deposits[tx.origin];
uint balance = balances[tx.origin];

if (now < depositTime + 7 days) {
     return false;
}
if (balance < amount){
    return false;
} 

if(msg.sender.call.value(balance)()) {
    balances[msg.sender] -= amount;
}

Transfer(this, msg.sender, amount);
return true;

}

function deposit(uint amount) returns(bool) {

balances[tx.origin]+=amount;
deposits[tx.origin]=now;

Transfer(this, msg.sender, amount);
return true;

}

}

pragma solidity ^0.4.4;

// This is a simple contract that keeps track of balances and allows withdrawals only after a week from the deposit.
// 1. In your opinion, does withdraw() function work as expected?
// 2. Implement the missing deposit() function that will allow only a single deposit from any address

contract MyContract {
mapping (address => uint) balances;
mapping (address => uint) deposits;

event Transfer(address indexed _from, address indexed _to, uint256 _value); // [HM]: indexed event to allow for listening to history of transactions

function withdraw(uint amount) returns(bool) { //would modify to not return bool, but throw error
// [HM]: would add an approval modifier from the ERC20 protocol for approved withdrawers

uint depositTime = deposits[tx.origin];
uint balance = balances[tx.origin];

// [HM]: would use the require statement (instead of 'if') to throw if this condition is not satisfied and log to console. 
// [HM]: Condition is incorrect. Should be a 'less than' logical check. Changing below. 
if (now < depositTime + 7 days) {
  return false;
}

// [HM]: would use the require statement (instead of 'if') to throw if this condition is not satisfied and log to console. 
if (balance <= amount) return false;

if(msg.sender.call.value(balance)()) {
  balances[msg.sender] -= amount; // [HM]:would use safemath here to avoid over/under flows
}

Transfer(this, msg.sender, amount); // [HM]:would abstract this and add the final transfer to a private function
return true;

}
function deposit(uint amount) {
//[HM]:same as previous function.. would add approval modifier based on ERC20 protocol

require(amount > 0);

balance[tx.origin] += amount; // [HM]:would use safemath here to avoid over/under flows
deposits[tx.origin] = now;

Transfer(msg.sender, this, amount) // [HM]:would abstract this and add the final transfer to a private function

}
}

// Hey! This is Saleh, I loved learning how to code in Solidity over the weekend. I found it similar enough to JS that it wasn't hard to pick up, but still needed to refer to documentation to understand certain things

// 1. In your opinion, does withdraw() function work as expected?
/*
No, I think there are a few logic problems:

1. The condition for the first if condition should change from if (now > depositTime + 7 days) To if (now < depositTime + 7 days). Since it should break if sooner (less than) 7 days from deposit.
2. The condition for the second if condition should change from if (balance <= amount) To if (balance < amount). Changed to allow for balance to equal amount.

*/

// 2. Implement the missing deposit() function that will allow only a single deposit from any address

contract MyContract {
  bool private deposited = false;

  /*
    rest of code as above
  */

  event Deposit (address indexed _from, bytes32 indexed _id, uint _value);
  
  function deposit(bytes32 _id) payable public {
      if (deposited = true) {
        revert();
      }
      emit Deposit(msg.sender, _id, msg.value);
      deposited = true;
  }
}

// I'm not a Solidity expert, so I'll wing it with some potential syntax problems and thanks for the opportunity.

// This is a simple contract that keeps track of balances and allows withdrawals only after a week from the deposit.
// 1. In your opinion, does withdraw() function work as expected?
// 2. Implement the missing deposit() function that will allow only a single deposit from any address

I agree with some of the analysis above.

// 1. In your opinion, does withdraw() function work as expected?

// No. There are some issues ...

if (now > depositTime + 7 days) {
  return false;
}

// this snippet incorrectly REFUSES a withdrawal AFTER the current time is MORE than 7 days after the deposit ... but those are not the requirements ... withdrawals should be ALLOWED only after a week has passed since the deposit. It should be ...

if (now < depositTime + 7 days) {
  return false;
}

// Further with withdrawal, you should be able to withdraw your ENTIRE balance ... not just some amount LESS than the balance. Therefore ONLY if the amount of the withdrawal is MORE than the balance should the withdrawal be refused.

if (amount > balance) return false;

// 2. Implement the missing deposit() function that will allow only a single deposit from any address

contract MyContract {

// As above

function deposit(uint amount) returns(bool) {

 require(amount > 0);

 balance[tx.origin] += amount; 
deposits[tx.origin] = now;

Transfer(this, msg.sender, amount);
return true;

}