Open atengberg opened 1 year ago
I'll end up with a list of suggestions like this. Anyways, for the comments section I'd suggest including information on how to generate "Motokodoc":
Immediately above a declaration, if the multiline comments start as /* and end as /, this will generate the tooltip on hover dsiplaying this comment elsewhere in the code. It is also used in when moc doc generation occurs. It has some features of markdown, such as enclosing * will bold, \ cause a new line break, two trailing spaces will also cause a new line break, single enclosing or _ cause italics, enclosing back ticks format for code, three enclosing back tacks format multline code. I typically try to break at 80 (at most 120) characters; and bold the first line as the header particularly if it is multiline.
Here's a a simple example:
/****Returns whether it is a valid account identifier's (the optional) subaccount.***/
public func isValidSubaccount(s : Subaccount) : Bool {
(s.size() == 32);
};
Which looks like:
I've seen @param used, but its not (currently) active syntax and there's no linking setup in the code (linking automatically does occur in generated docs to references) afaik so I personally don't use that; if in a Types.mo file I'll just add those fields as part of the comment so it'll appear when hovering on it elsewhere in the codebase (see below) to make it easier to parse unfamiliar code. I also do not leave the leading * on new lines since they are not formatted out of the tooltip (and possible gen'd docs) (these are sometimes autogenerated).
Here's another slightly more involved example:
/****`err` result type of method `remove_allowed_creator`.**
Has field:
-`kind : {`
`#NotAuthorized`
`#NotFound`
`}` */
public type RemoveAllowedCreatorErr = {
kind : {
#NotAuthorized;
#NotFound;
};
};
Another disclaimer bulletin point, might be pointing out that the ICP ledger and ICRC1 standard have custom Result types where the #ok and #err tags are capitalized instead of lower case (as this causes functional incompatibility if not declared correctly):
`public type Result<T, E> = {
};`
I'm not sure the reason, but it's one of those "gotchas" that would be suitable for such a disclaimer bulletin.
In the Variants section, perhaps include the terminology "tag" and tag's "argument" to specify the members of a variant's declaration--at least, I've seen these terms used a lot in the context of variants. Reference to variants as a sum type might also be useful, or as a footnote for further exploration/discussion.
While on this subject, is the Option type a special case of a (binary) generic variant (at the two ends of boundedness, any and null)? Might be useful along with Result as introducing generic variants.
Are objects equivalent to record literals?
I was under the impression they are more like singletons, and distinct from record literals. In any case, my suggestion is to add a point of consideration in the Classes section, as when thinking about classes, as they are not like OO classes, specifically they are more expensive to create/use which is an important consideration for a smart contract... I don't know the details of this as well as I'd like, but a general rule I now check myself with is asking if the class can be substituted for an object/if I really need the class at all.
On this note, it also might be worth mentioning about "non-static expressions" in modules (one time I specifically do use a class in a development environment).
On an unrelated note, it might be worthwhile to add an explanation of a do{}
block expression in the Functions section, as it is kind of like an anonymous inline function. Might be a place to introduce the do?{}
block expression as well (apologies for misusing "block expression", I think it's supposed to be one or the other).
(Sorry for all the comments lol). Anyways last comment for tonight: add a section on Unit testing with Motoko matchers. TDD should be the way any smart contract programming should be done, and in the future when code snippets can be indexed and normalized in various ways for processing (an example being such as how Viper does code auditing), will make doing that a lot easier for everyone's benefit, besides the very real immediate benefits it brings.
Thank you so much! Many points I havent thiught about.
Will add many of your suggestions and will respond to others.
😀
Hope to get to this feedback next week! Excuse the delay
Result
(#ok vs #Ok) in ICP ledger vs ICRC1Are objects equivalent to record literals?
No, I only mention that records are object literals. Is that correct?
I was under the impression they are more like singletons, and distinct from record literals. In any case, my suggestion is to add a point of consideration in the Classes section, as when thinking about classes, as they are not like OO classes, specifically they are more expensive to create/use which is an important consideration for a smart contract... I don't know the details of this as well as I'd like, but a general rule I now check myself with is asking if the class can be substituted for an object/if I really need the class at all.
On this note, it also might be worth mentioning about "non-static expressions" in modules (one time I specifically do use a class in a development environment).
This is mentioned here, you might have missed it.
On an unrelated note, it might be worthwhile to add an explanation of a
do{}
block expression in the Functions section, as it is kind of like an anonymous inline function. Might be a place to introduce thedo?{}
block expression as well (apologies for misusing "block expression", I think it's supposed to be one or the other).
I've noticed the fact let is for immutable variables which is somewhat the opposite of its use in Javascript, to be a source of confusion.
I'd suggest adding a "Caution!" bulletin point explicitly stating it is not like how it is used in Javascript for the sake of clarity. This might seem obvious to someone who has experience programming, but this might not always be the case for each person learning Motoko.