Skip to content
This repository has been archived by the owner on Apr 13, 2022. It is now read-only.

Pull Request for Generic Boxes Issue #23 #25

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

zihehuang
Copy link

@zihehuang zihehuang commented Jun 5, 2017

Pull request for Issue #23. I took the liberty of converting Box into a generic type. I had to take validate out of BoxMinimalState since the implementation requires the Box's value type to be a Long.

@zihehuang zihehuang mentioned this pull request Jun 5, 2017
@@ -6,14 +6,9 @@ import scorex.core.transaction.box.proposition.Proposition
/**
* Box is a state element locked by some proposition.
*/
trait Box[P <: Proposition] extends BytesSerializable {
val value: Box.Amount
trait Box[P <: Proposition, T] extends BytesSerializable {
Copy link
Contributor

Choose a reason for hiding this comment

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

What if, instead of making Box more generic by changing the type of value from long to T and carrying T around as a type parameter, we made Box more generic simply by removing value entirely? It would be up to subclasses of Box to have fields that contain content (e.g. values or anything else one might want to add to the box).

@kushti @catena2w @zihehuang , what do you think?

Copy link

Choose a reason for hiding this comment

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

I know this is a bit old, but that could likely work just fine based on our discussion for #23 . @zihehuang probably had the same mindset I did when we were working on this locally. It's then easy enough to create a parameterised ValueBox if you wanted that field, anyway.

catena2w added a commit that referenced this pull request Jun 13, 2018
Base automatically changed from master to main February 3, 2021 23:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants