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

Possible small code errors in listings 5 and 11? #12

Open
masonhieb opened this issue May 24, 2024 · 4 comments
Open

Possible small code errors in listings 5 and 11? #12

masonhieb opened this issue May 24, 2024 · 4 comments

Comments

@masonhieb
Copy link

On this line here in listing 11, shouldn't tree.y = depth be at the top of the setup function?

I was implementing this recently and, with the function written as shown, I believe that code would only set the y values for trees with no children, but it should be setting the y values for every tree, should it not? Or is there an assumption here that the y-values are already set elsewhere (since most of the real complexity comes from the x coordinates, not the y coordinates)?

Additionally, in listing 5 this tree.offset class variable is used but seems to be accidentally copied over from listing 4. Should that value instead be tree.mod?

@llimllib
Copy link
Owner

I don't know, unfortunately! This code was written 17 years ago and the drawings were done on a piece of software that no longer exists.

I would love to apply corrections, but I'm not sure that I know how to verify them any longer; I'm open to your opinions on how best to proceed.

@llimllib
Copy link
Owner

llimllib commented May 28, 2024

Okay, I've started a typescript translation.

Definitely in listing 5 either line 44 should be tree.mod or line 40 should be tree.offset, will deal with listing 11 when I get there

@masonhieb
Copy link
Author

Ok, thanks for taking a look! I implemented listing 11 in C++ and only the childless nodes of my tree were at the correct y-coordinate. I hope you'll still keep the original up somewhere if your plan is to create a more modern version.

@llimllib
Copy link
Owner

definitely. Thanks for filing this issue!

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

2 participants