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.

Sign up for GitHub

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

Jump to bottom

Refactor trickle DAG builder #4730

Merged
merged 1 commit into from
Mar 23, 2018
Merged

Refactor trickle DAG builder #4730

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 18 additions & 19 deletions importer/trickle/trickledag.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,9 @@ const layerRepeat = 4
// explanation.
func Layout(db *h.DagBuilderHelper) (ipld.Node, error) {
root := db.NewUnixfsNode()
if err := db.FillNodeLayer(root); err != nil {
if err := fillTrickleRec(db, root, -1); err != nil {
return nil, err
}
for level := 1; !db.Done(); level++ {
for i := 0; i < layerRepeat && !db.Done(); i++ {
next := db.NewUnixfsNode()
if err := fillTrickleRec(db, next, level); err != nil {
return nil, err
}
if err := root.AddChild(next, db); err != nil {
return nil, err
}
}
}

out, err := db.Add(root)
if err != nil {
Expand All @@ -65,25 +54,35 @@ func Layout(db *h.DagBuilderHelper) (ipld.Node, error) {
return out, nil
}

func fillTrickleRec(db *h.DagBuilderHelper, node *h.UnixfsNode, depth int) error {
// fillTrickleRec creates a trickle (sub-)tree with an optional maximum specified depth
// in the case maxDepth is greater than zero, or with unlimited depth otherwise
// (where the DAG builder will signal the end of data to end the function).
func fillTrickleRec(db *h.DagBuilderHelper, node *h.UnixfsNode, maxDepth int) error {
// Always do this, even in the base case
if err := db.FillNodeLayer(node); err != nil {
return err
}

for i := 1; i < depth && !db.Done(); i++ {
for j := 0; j < layerRepeat && !db.Done(); j++ {
next := db.NewUnixfsNode()
if err := fillTrickleRec(db, next, i); err != nil {
for depth := 1; ; depth++ {
Copy link
Member

Choose a reason for hiding this comment

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

Code nit: You can write this as for depth := 1; depth < maxDepth || maxDepth < 0; depth++ {.

Copy link
Contributor Author

@schomatis schomatis Mar 2, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I put it separately to highlight the fact that maxDepth is an optional parameter (that will most commonly be unset, which is the base case) but I can move it all together to make it more compact if you think it's more in line with Go coding principles.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds reasonable. I don't feel strongly either way.

// Apply depth limit only if the parameter is set (> 0).
if maxDepth > 0 && depth == maxDepth {
return nil
}
for layer := 0; layer < layerRepeat; layer++ {
if db.Done() {
return nil
}
Copy link
Member

Choose a reason for hiding this comment

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

Why move do this separately?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid repeating the same condition in both fors, and make it more clear that if db.Done is true nothing more needs to be done in this function and it should just exit there (removing the final return from the code).


nextChild := db.NewUnixfsNode()
if err := fillTrickleRec(db, nextChild, depth); err != nil {
return err
}

if err := node.AddChild(next, db); err != nil {
if err := node.AddChild(nextChild, db); err != nil {
return err
}
}
}
return nil
}

// Append appends the data in `db` to the dag, using the Trickledag format
Expand Down