Skip to content

Changed List to Pair, and also re-implemented type_of#3

Open
wongjiahau wants to merge 5 commits intomasterfrom
change-list-to-pair
Open

Changed List to Pair, and also re-implemented type_of#3
wongjiahau wants to merge 5 commits intomasterfrom
change-list-to-pair

Conversation

@wongjiahau
Copy link

No description provided.

@wongjiahau wongjiahau requested a review from zypeh February 14, 2020 02:48
Copy link

@hch12907 hch12907 left a comment

Choose a reason for hiding this comment

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

I like the new type_of().

src/node.rs Outdated
pub fn cdr(node: Node) -> Result<Node, &'static str> {
match node {
Node::Pair(_, b) => Result::Ok(*b),
_ => Result::Err("Cannot apply cdr to Pair")

Choose a reason for hiding this comment

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

I think it's better to use a dedicated Error enum instead of a string for Result::Err... but I digress

Choose a reason for hiding this comment

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

However, for this case Option<T> is better as your return type

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.

3 participants