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

Caleb/dev #28

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

Caleb/dev #28

wants to merge 8 commits into from

Conversation

CT-Kyle
Copy link

@CT-Kyle CT-Kyle commented Oct 13, 2016

Name: Caleb Kyle
SMU ID: 38045609

Question: On the fourth test of ex03, sometimes it will pass, other times it won't pass. Does this mean that I'm only using one core and sometimes the 2 chunk process just finishes faster? Why is it not consistently one way or the other?

@pragdave
Copy link
Contributor

The code runs OK here, and looks good to me. It might be your machine.

def counter(value \\ 0) do
receive do
Copy link
Contributor

Choose a reason for hiding this comment

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

-2 layout

The indentation of line 34 is deceptive. It's really part of the {:next, from} handler, and should share indentation with line 33.

def new_counter(value) do
Agent.start_link(fn -> value end)
end
def next_value({:ok, counter}) do
Copy link
Contributor

Choose a reason for hiding this comment

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

-4 idiomatic

new_counter currently returns {:ok, pid}, which is why you have to pattern match in next_value. But the :ok part is only used inside new_counter—it shouldn't leak out. So write something like

def new_counter(value) do
  { :ok, count } = Agent.start_link(fn -> value end)
  count
end

def pmap(collection, process_count, function) do
« your code here »
chunk_size = Enum.count(collection) / process_count
Copy link
Contributor

Choose a reason for hiding this comment

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

30

fyi: you could use div here, and replace map |> concat with flat_map.

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.

2 participants