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

Update memo struct to use additional structure as memo item instead of embedded structure #77

Merged
merged 1 commit into from
Sep 6, 2023

Conversation

dzmitryhil
Copy link
Contributor

With the previous embedded struct, it was convenient if you read the transaction, but if not convenient if you create a transaction with the memo.

Previous ways of creating a memo:

type MyMemoItem struct {
	MemoType   data.VariableLength
	MemoData   data.VariableLength
	MemoFormat data.VariableLength
}

func main() {
	// example 1
	memos := make(data.Memos, 1)
	memos[0].Memo.MemoType = data.VariableLength("type")
	memos[0].Memo.MemoData = data.VariableLength("data")
	memos[0].Memo.MemoFormat = data.VariableLength("format")

	// example 2
	memos = []data.Memo{
		{
			Memo: struct {
				MemoType   data.VariableLength
				MemoData   data.VariableLength
				MemoFormat data.VariableLength
			}{
				MemoType:   data.VariableLength("type"),
				MemoData:   data.VariableLength("data"),
				MemoFormat: data.VariableLength("format"),
			},
		},
	}

	// example 3
	memos = []data.Memo{
		{
			Memo: MyMemoItem{
				MemoType:   data.VariableLength("type"),
				MemoData:   data.VariableLength("data"),
				MemoFormat: data.VariableLength("format"),
			},
		},
	}
}

The proposed changes are not breaking for example 1 and example 2, but breaking for example 3 which is rare in terms of usage in general. Anyway IMHO, better practice to have a common MemoItem.

memos = []data.Memo{
		{
			Memo: data.MemoItem{
				MemoType:   data.VariableLength("type"),
				MemoData:   data.VariableLength("data"),
				MemoFormat: data.VariableLength("format"),
			},
		},
	}

@dzmitryhil dzmitryhil changed the title Update memo struct to use and additional structure as memo item instead of embedded structure Update memo struct to use additional structure as memo item instead of embedded structure Sep 6, 2023
@donovanhide donovanhide merged commit 7739c2f into rubblelabs:master Sep 6, 2023
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants