Skip to content

Conversation

@eltNEG
Copy link
Contributor

@eltNEG eltNEG commented Jan 20, 2020

Description

It fixed the outdated contract in sophia-vote-contract.md. It also replaces all references to forgae with aeproject.

Tasks completed

  • Fix the Sophia contract in sophia-vote-contract.md
  • Replaces references to forgae with aeproject

Notes

  • None

Copy link
Collaborator

@thepiwo thepiwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, added some comments

votes[candidate] = { voters = [] } })
private function look_up_by_address(k : address, m, v) =
function lookup_by_address(k : address, m, v) =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can be replaced with Map.lookup_default(k : 'k, m : map('k, 'v), v : 'v) : 'v

We need to make use of a `size` function which we define as a helper function below. Here is the code:
`````
private function size(l : list('a)) : int = size'(l, 0)
function size(l : list('a)) : int = size'(l, 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The size function is now included with the standard library

include "List.aes"

https://github.com/aeternity/protocol/blob/master/contracts/sophia_stdlib.md#length

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback. I tried a couple of syntaxes which returns unbound variable... error

Tried syntaxes

List.length(state.votes[candidate].voters)
length(state.votes[candidate].voters)
List.size(state.votes[candidate].voters)
size(state.votes[candidate].voters)

Kindly assist with this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I failed to `include "List.aes". It is working now.

let new_votes_state = Call.caller :: state.votes[candidate].voters
put(state{
votes[candidate].voters = new_votes_state })
true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guess there is no need to return true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback. I have removed it. Kindly, resolve the conversation if you are satisfied with the new changes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still seems to be there

@thepiwo
Copy link
Collaborator

thepiwo commented Jan 20, 2020

maybe @DanielaIvanova can check as well :)

votes: map(address, candidates) }
public stateful function init() : state = {
stateful entrypoint init() : state = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function does nothing stateful so the keyword can be removed.

Copy link
Contributor Author

@eltNEG eltNEG Jan 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I have implemented the requested changes. Can you resolve the conversation?

public stateful function vote(candidate: address) : bool =
stateful entrypoint vote(candidate: address) : bool =
is_candidate(candidate)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is convoluted, you better use a default value when looking up the candidate and do away with the is_candidate function. I.e. something like this:

stateful vote(candidate: address) : unit = 
  let current_votes = state.votes[candidate = {voters = []}].voters
  put(state{ votes[candidate] = {voters = Call.caller :: current_votes }})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a dilemma here. With this change, there will be no need for add_candidate function. Can you give me feedback on the new full contract? I will change the wordings in the tutorial when the contract is satisfactory.

cc: @thepiwo ,

_ :: l' => size'(l', x + 1)
public stateful function add_candidate(candidate: address) : bool =
stateful entrypoint add_candidate(candidate: address) : bool =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the point in always returning true?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is my first time using a language without a return statement. I think it's a reaction to the weird feeling 🙈. Thanks, I have removed them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have implemented the requested changes. Can you resolve the conversation?

switch(l)
[] => x
_ :: l' => size'(l', x + 1)
function size'(l : list('a), x : int) : int =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With compiler v4.2.0 this can be written more nicely as:

function 
  size' : (list('a), int) => int
  size'([], x)     = x
  size'(_ :: l, x) = size'(l, x + 1)

But as @thepiwo mention it is a standard library function.

Copy link
Contributor Author

@eltNEG eltNEG Jan 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have implemented the requested changes. Can you resolve the conversation?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still seems to be there in the explaination

Copy link
Contributor Author

@eltNEG eltNEG Jan 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I want the contract reviewed before I change the content of the tutorial.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is any need to document size any more

First we have to initialize our project where we write our smart contract. In order to do this we are going to use `aeproject init`.

## Smart contract
In Sophia ML we have a state which is the place to store data on-chain - and it is the only part in the smart contract that can be mutated (overwritten).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
In Sophia ML we have a state which is the place to store data on-chain - and it is the only part in the smart contract that can be mutated (overwritten).
In Sophia we have a state which is the place to store data on-chain - and it is the only part in the smart contract that can be mutated (overwritten).

Copy link
Contributor Author

@eltNEG eltNEG Jan 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have implemented the requested changes. Can you resolve the conversation?

@eltNEG eltNEG force-pushed the fix/sophia-vote-contract branch from de88f12 to 630dab6 Compare January 21, 2020 13:58
None => v
Some(x) => x
`````````
Map.lookup_default(k, m, v)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the whole function can be removed and replaced

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@eltNEG eltNEG force-pushed the fix/sophia-vote-contract branch 2 times, most recently from ceb45b3 to b2d508a Compare January 21, 2020 17:02
@eltNEG eltNEG force-pushed the fix/sophia-vote-contract branch from b2d508a to a6f3c2e Compare January 29, 2020 11:13
@eltNEG eltNEG force-pushed the fix/sophia-vote-contract branch from a6f3c2e to 9d8af93 Compare January 29, 2020 11:18
@Pegahbit Pegahbit merged commit 632af5f into aeternity:master Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants