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

chap02 ex1 better implement #31

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

amoszhou
Copy link

@amoszhou amoszhou commented Apr 1, 2016

I think this is more better

@ryblovAV
Copy link
Contributor

ryblovAV commented Apr 2, 2016

Hi @amoszhou,

a: =>A and b: =>B are by-name-parameters (not lazy values), they are evaluated every time when
they are used
In your solution they are executed twice

@amoszhou
Copy link
Author

amoszhou commented Apr 2, 2016

@ryblovAV yes you are right.
I just make the code looks like simple,! thanks you

@ryblovAV
Copy link
Contributor

ryblovAV commented Apr 2, 2016

@amoszhou
You're welcome

t1.join()
t2.join()

(aVal, bVal)
(a, b)
Copy link
Member

Choose a reason for hiding this comment

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

@amoszhou Commeny from @ryblovAV is correct - the expression a is by name (not lazy), and it will be executed twice now. The first time in parallel in the thread statement, and the second time when returning from the method. This effectively removes parallelism, and if the a or b are side-effecting, will execute the side-effect twice.

If you want nicer code, you could do this:

lazy evaluatedA = a
lazy evaluatedB = b

val t1 = thread {
  evaluatedA
}
...
(evaluatedA, evaluatedB)

Copy link
Author

Choose a reason for hiding this comment

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

yes, thank you

Copy link
Member

Choose a reason for hiding this comment

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

You are welcome, just don't forget to add lazy vals and push again to sync the pull request if you made any changes.

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