JoshOrndorff / librho

A Standard Library for Rholang
Apache License 2.0
12 stars 6 forks source link

Style guide #7

Closed JoshOrndorff closed 5 years ago

JoshOrndorff commented 6 years ago

No one has written one, it may as well be us. I'll put my suggestions each in their own comment so we can express feedback with reactions. Counter arguments are also welcome.

@dckc @crypto-coder @Eknir @differentialDerek

JoshOrndorff commented 6 years ago

Indent with two spaces

JoshOrndorff commented 6 years ago

For single line expressions, trailing |s go on the same line after a single space myChan!(myProc) |

JoshOrndorff commented 6 years ago

For ending lexical scopes, pars go on a separate line

for (_ <- myChan) {
  // Do something
}
|
JoshOrndorff commented 6 years ago

System powerbox channels are given the short name of their URI

new stdout(`rho:io:stdout`)
JoshOrndorff commented 6 years ago

When sending a simple acknowledgement that carries no additional information, send Nil

JoshOrndorff commented 6 years ago

Contracts that are essentially used as call and return functions take one channel for successful results, called return and one for error messages and diagnostics called error.

contract isPositive(@n, return, error) = {
  match n {
    Int => { // send something on return }
    _ => error!("Only integers allowed in isPositive")
  }
}
dckc commented 6 years ago

I'm pretty sure you don't want the results of a function call coming back on two separate channels. Can't put my finger on why just now. But I suggest: use a Maybe (simulated with [] for Nothing and [x] for Just x) or Either type (using a tagged tuple ('ok', result) or ('err', reason)).

JoshOrndorff commented 6 years ago

I was thinking of this like javascript promises where they can resolve or error out. Having separate channels for success vs error allows.

  1. The contract that errors need not return that error to its immediate caller who will likely pass it on to its own caller the way it works in a traditional call stack. Instead stderr or whoever ultimately needs to know about the error can be immediately informed.
  2. It avoids the pattern
    if (test for errors){
    // Handle errors
    }
    // Finally do useful stuff

    and allows

    
    for (x <- success) {
    // Do useful stuff
    }

for (e <- error) { // Handle errors }

JoshOrndorff commented 6 years ago

Channels that are used as data-stores should be named after the data they hold plus a Ch at the end

new nameCh in {
  nameCh!("Joshy") |
  for (@name <- nameCh) {
    //Do useful stuff
  }
}
Eknir commented 6 years ago

Use and @ sign in a for comprehension when you only intend to use the received name as a process (and don't intend to send something on it)


// n will be used only as a process in the contract isPositive
// return and error will be used to send something on it (but perhaps also as a process)
contract isPositive(@n, return, error) = {
  match n {
    Int => { // send something on return }
    _ => error!("Only integers allowed in isPositive")
  }
}```
Eknir commented 6 years ago

Names and processes are written in lowerCamelCase

Eknir commented 6 years ago

Question: In which cases do we prefer to use contract instead of for(_ <= someCh){ }

glenbraun commented 6 years ago

Change example to this below. Use of the word 'name' is overloaded.


new firstNameCh in {
  firstNameCh!("Joshy") |
  for (@firstName <- firstNameCh) {
    //Do useful stuff
  }
}
glenbraun commented 6 years ago

It seems strange to use "Ch" indicating channel when it seems more like the usage here is of a variable, so maybe firstNameData or firstNameVar, or varFirstName, is a better convention (or similar).

When I think channel, I think that there is a flow or stream of values that are different from each other. Maybe fistNameCell is a good convention. It means that the data which is a first name will be maintained on that name. That it is expected that there will be one, and only one, pending read on the channel and what is read is going to be the data, and than one had better write back the value or it will be lost. (I know, in this case, Cell, might not be good because there is a Cell example where one uses get and set capabilities so there is no way to use the thing and loose the value.) So, maybe Cell isn't the right suffix either, though I kind of like it, or varFirstName is looking pretty good too.

If we use 'Ch' what would we call a name that is a channel/stream of, perhaps, arbitrary terms?

What are the patterns of usage for a name: 1) as an identifier (bundle0(n)) 2) as a storage for data (what we're calling 'Ch' here) 3) as a stream of terms (???) 4) as a stream of terms of some type of data (???) 5) as a stream of terms which are intended to be evaluated (???) 6) as a this pointer? etc. ?

Eknir commented 6 years ago

When we define a send and receive concurrently, we first define the send process after which we define the receive.

dhsorens commented 6 years ago

Question: In which cases do we prefer to use contract instead of for(_ <= someCh){ }

That's a good question; my impression is that when you write a smart contract, the outermost process is a contract and not a persistent listen for( .. <= ... ){ }. Not totally sure tbh.

glenbraun commented 6 years ago

I've been playing with never using contract and always using for (x <= ch). I've never liked multiple ways to do the same thing. I know the contract syntax looks more like a function syntax but I could imagine a time when Rholang really does have functions and so the contract syntax would be more confusing than now.

dckc commented 6 years ago

Channels that are used as data-stores should be named after the data they hold plus a Ch at the end

but then the example consumes data on the channel without putting it back. That's not using it as a store, is it? Did you mean to put it back inside the for in the example? (i.e. use peek, if we had it)

FWIW: In https://github.com/rchain/rchain/pull/1265/files , @pyrocto uses verifyStore for such things and guardedBallotOut in other cases. Birch seems to use the xCh idiom.

glenbraun commented 6 years ago

Regarding object capabilities, see https://github.com/JoshOrndorff/LearnRholangByExample/tree/master/9-ObjectCapabilities

The example has the names for the methods sent into a factory, which implied, to me, that it was this pattern of the names for methods being sent in from the outside which signaled an object capabilities design approach. I'm thinking that holding a name and being able to send on it, receive on it, whatever (based on bundle) is a capability regardless of who did the 'new' to create the name.

See https://github.com/glenbraun/LTP/blob/master/CreateTable.rho which wraps the capability to get, add, etc. to a table in a license. Only if there are sufficient funds can the person get, add, etc. I consider this using object capabilities.

glenbraun commented 6 years ago

@dckc, regarding channels used as data-stores. Yes, I did mean that the data would be put back on the name. Yes, with peek, we can access these variables and only replace the value on an update. I've been using the style, 'varLastName', or 'varBalance' and it looks meaningful to me. With this naming convention I know that there is meant to be only one item ever on this name, what that one value is meant to represent, and know that if I pull the value off it, then I must put it back. Peek will make this nicer.

Isaac-DeFrain commented 5 years ago

Names and processes are written in lowerCamelCase

How about names are written in lowerCamelCase and processes are written in UpperCamelCase?

JoshOrndorff commented 5 years ago

I'm ready to write this up into a more proper style guide document. Well a working draft to make PRs against anyway. Let's take until Monday to cast final votes and express final thoughts. Pass it along to anyone who may be interested.

@Isaac-DeFrain @dckc @crypto-coder @glenbraun @David405 @theophoric @tschoffelen @Eknir @differentialderek @EdEykolt @pdonorio @JeremyBeal Other people I would like to include but don't know github names, please tag them if you can.

ArturGajowy commented 5 years ago

The | separating parallel statements should always follow at the same line the statement ends. This means, for single-line statements (as seen above)

ch!("Hello ") |
ch!("World!")

For multi-line statements / blocks:

for (_ <- myChan) {
  // Do something
} |
ArturGajowy commented 5 years ago

re: error handling:

I was going to say that I'm totally with @dckc on having a single return channel for both success and error results. Otherwise you need additional logic to determine whether the call was finished (it had either error or success) or not.

But it is not a problem - both approaches are easily adapted into the other by (de)multiplexing. Moreover, multiplexing from 2 channels into 1 is easier than the other way around (doesn't require a match).

In the single-channel approach, you get one response whenever the call finished, whether it's success or not. But if that is needed - use multiplexing.

I guess the 'leave error handling to the upstream caller' is what makes the two channel approach win for me.

JoshOrndorff commented 5 years ago

I wrote this up in 56240a4f5a4097e63e45b42a414d5d12ca7d6a05. Thank you all for your help!

It's very much a first attempt, so I welcome further ideas. Let's open an issue or PR for each suggestion to keep focused.