Remove Feed Builder in favor of Feed simple create function

[FreddieRidell]

hypercore/src/feed_builder.rs

Lines 14 to 18 in 482f491

	/// Construct a new `Feed` instance.
	// TODO: make this an actual builder pattern.
	// https://deterministic.space/elegant-apis-in-rust.html#builder-pattern
	#[derive(Debug)]
	pub struct FeedBuilder<T>

The link referenced shows a crate that offers a macro to automatically derive builders for structs. It seems like it should be relatively easy to use derive_builder with Feed. Is the intention to use this macro, or write a custom builder?

[yoshuawuyts]

@FreddieRidell I was thinking a custom builder, as we're able to have more control over it.

[FreddieRidell]

Looking through the FeedBuilder, it seems that byte_length should be being set to the total number of bytes already recorded in the Storage? this can be done, but would require updating to the newest versions of update-random-access-*.

Have I understood this correctly?

[yoshuawuyts]

it seems that byte_length should be being set to the total number of bytes already recorded in the Storage

Though my memory is a bit foggy on the matter, I believe that's accurate.

[ZhouHansen]

Is there an actual builder pattern? I tried to make it an Non-consuming builder, then we can write:

if partial_keypair.secret.is_some() {
    builder.secret_key(partial_keypair.secret.unwrap())
}

Ok(builder.build()?)

But the lifetime doesn't allow it. But it is ok:

Under the rubric of making easy things easy and hard things possible, all builder methods for a consuming builder should take and returned an owned self.

Builder-pattern is useful when build a object takes a lot of work. Maybe we can build feed directly here? I opened a pr for it.

[yoshuawuyts]

@ZhouHansen I'm not sure I follow. The feed_builder is an "actual" builder. The way to write the code above is:

if partial_keypair.secret.is_some() {
    builder = builder.secret_key(partial_keypair.secret.unwrap())
}

Ok(builder.build()?)

Maybe I'm missing something. But whatever the problem is, it seems unlikely that removing the builder in its entirety would be the solution.

[ZhouHansen]

@yoshuawuyts I see the // TODO: in the comment, this misleads me that the current feed_builder is not an "actual" builder yet and there're more works to do here.

At the same time I feel it's unnecessary to use builder-pattern here, the building process of feed is not complex.

[bltavares]

As part of the discussion, it seems like the Builder is an actual builder.

In regards to the Feed being simple: check my comment on the PR #80 (comment)