Improve documentation for memory and table grow methods - #122
Conversation
| // Implementation note: try-catch is faster than explicit checks and can be optimized by the | ||
| // JVM. Catching generic RuntimeException to keep the method bodies short and easily inlinable. |
There was a problem hiding this comment.
Moved this here, because it is probably not relevant in user-facing Javadoc.
andreaTP
left a comment
There was a problem hiding this comment.
LGTM, thanks a lot!
For the records: I, personally, don't like JavaDocs(several reasons...), but I know that is a ship that should eventually sail.
You are introducing those with focused changes and this makes it for smoother reviews (and personal acceptance of the fact), thanks so much.
| // Implementation note: try-catch is faster than explicit checks and can be optimized by the | ||
| // JVM. Catching generic RuntimeException to keep the method bodies short and easily inlinable. |
|
I also don't think documenting everything is necessary; often things are obvious (getters, setters) or redundant or duplicated (with But on the other hand Javadoc can also be quite useful I think, especially when describing the purpose of a class or method and how it interacts with other classes, and when describing things to look out for such as special or error behavior or argument restrictions. |
I agree, and I acknowledge we should do better in this regard in this project. At the same time I go mad when I see drift happening(i.e. comments misaligned with implementation) 😅 |
I think especially the "return -1 on error" is an important aspect to document.
Side note: The JavaScript API seems to throw an exception for errors. Though I understand that the Endive implementation here is used both as API and as implementation for the Wasm opcode (which requires -1 on error). Not sure how often these
growmethods are used from the API by users, but maybe it would make sense then in the future to consider refactoring it to throw an error by default instead of using -1 as return value (respectively having atryGrowwhich returns -1)?