Skip to content

Fix a little bug in the example code #1583

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

Merged
merged 1 commit into from
Dec 15, 2019
Merged

Fix a little bug in the example code #1583

merged 1 commit into from
Dec 15, 2019

Conversation

qiuyangnie
Copy link
Contributor

The variable (a, b, c) in each case should be matched the variable printed out.

The variable (a, b, c) in each case should be matched the variable printed out.
@SethTisue
Copy link
Member

SethTisue commented Nov 20, 2019

@qiuyangnie I agree this is an improvement; the current code is just plain wrong

however, I wonder if it a different fix might be better: namely, to write case a all three times. the current code might give the reader the impression that it's necessary to make up a different name each time. but it isn't necessary.

wdyt?

@qiuyangnie
Copy link
Contributor Author

qiuyangnie commented Nov 20, 2019

I do agree with you! The earlier example in this chapter, however, should have demonstrated the 'variable name' in each case can be same. This example could provide an alternative view to demonstrate the 'variable name' in each case can be different.

@SethTisue
Copy link
Member

SethTisue commented Dec 6, 2019

I think that's a subtlety that would be lost on readers unless it was explicitly called out, and I don't think it's actually worth calling out, because using a different name each time doesn't actually make the code better; I think it's actually a bit worse that way.

So I still suggest that the right fix here is simply:

write case a all three times

@SethTisue
Copy link
Member

SethTisue commented Dec 6, 2019

(regardless: if you disagree or simply aren't interested in pursuing this further, I'll hit "merge" anyway)

@SethTisue SethTisue merged commit 27a4d87 into scala:master Dec 15, 2019
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