Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add style guide updates based on CS19 notes #46

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ZacharyEspiritu
Copy link
Contributor

CS19 has a few style guide updates based on our grading rubrics—we're looking to shift to linking to the Pyret style guide as opposed to our own so we can declutter the website a bit, so if we could add these changes to the pyret-docs style guide, that would be great!

@ZacharyEspiritu
Copy link
Contributor Author

(Sorry, my text editor automatically removed trailing whitespace changes which is where some of the extra lines in the diff come from.)


@codedisp{
fun is-this-noah(str :: String) -> String:
Ask:
Copy link
Contributor

Choose a reason for hiding this comment

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

ask

@sorawee
Copy link
Contributor

sorawee commented Sep 5, 2018

Sorry, didn't meant to use the GitHub review feature. Just want to leave comments.

fun search<A>(lst :: List<A>) -> A: ... end
}

Now, the elements of @tt{lst} can be of any type, but all elements of lst will
Copy link
Member

Choose a reason for hiding this comment

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

Bad wording -- you just got through saying you don't want to have a list of Any type, but it's ok now to have a list of any type? (Note the capitalization...) You mean to say something closer to "Now, we don't know or care what the type of the elements of lst are, but we are asserting that they all be of the same type -- namely A. This distinction (between "any type at all", and "any single type, for any particular list") is subtle but important."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reworded it according to your comments—let me know what you think!

important distinction between "any type at all" (@tt{List<Any>}) and
"any single type, for any particular list" (@tt{List<A>}).

Note that the actual name of the type parameter is arbitrary (we could have
Copy link
Contributor

@sorawee sorawee Sep 5, 2018

Choose a reason for hiding this comment

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

I believe recently @jpolitz just changed the documentation for fold so that type variables are not a single letter, which might invalidate this paragraph.

In other words, @tt{[list: 1, "apple", true]} would satisfy the annotation
@tt{List<Any>}. Creating @tt{List}s like this is usually bad practice.

In Pyret, we can do this by parametrizing the type. Rather than:
Copy link
Contributor

Choose a reason for hiding this comment

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

parameterizing? According to https://en.wikipedia.org/wiki/Parametric_polymorphism:

We say that the type of append is parameterized by a for all values of a


Similarly, do not use an @tt{if} expression to evalulate a predicate, and
then return @tt{true} or @tt{false}. For example, this would be
considered incorrect:
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is about style, I feel it's weird to say that something is incorrect. Perhaps use "inappropriate" (or another word) instead?

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