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

pov: why is root not in leaves or otherwise used? #847

Closed
shaleh opened this issue Sep 8, 2017 · 6 comments
Closed

pov: why is root not in leaves or otherwise used? #847

shaleh opened this issue Sep 8, 2017 · 6 comments

Comments

@shaleh
Copy link

shaleh commented Sep 8, 2017

The sequence is:

graph := New()
for _, l := range leaves {
    graph.AddNode(l)
}
for _, p := range arcPairs {
    graph.AddArc(p.from, p.to)
}

No where in there is the value for "root" added. So when AddArc is called the value of from is unknown.

The singleton test which is the first test has one leaf which matches the value of root. Most of the others use "parent" which is never explicitly defined.

The comments in the test file say that to will always exist but is quiet on from. It seems odd that the expectation is to create missing from. Or did I missed something?

@tleen
Copy link
Member

tleen commented Sep 10, 2017

If it is unclear to you than it is probably unclear and we should fix.

@robphoenix
Copy link
Contributor

@shaleh do you have a fix for this?

I have to admit I don't fully understand what the issue is, would you be able to expand on it so we can outline what a fix for this might look like. Thanks.

@ferhatelmas
Copy link
Contributor

Actually, every node is root. IMHO, it's the point of the exercise. It changes according to where you're looking from.

However, you're right. Current implementation has hidden assumptions and isn't aligned with problem canonical data

Anyway, we will need to write a generator for it so we can rework. @shaleh do you want to give a try ?

@shaleh

This comment has been minimized.

@ferhatelmas

This comment has been minimized.

@junedev
Copy link
Member

junedev commented Oct 23, 2021

Since this will be solved by writing the generator and that is tracked in another issue (#605) I am closing this one.

@junedev junedev closed this as completed Oct 23, 2021
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

No branches or pull requests

6 participants