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

feat(list): Add TransformList and NewListTransformed to aid in conversions #50

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ashquarky
Copy link
Member

Lists of RVTypes can be difficult to feed into other libraries. For example, List[String] shows up in DataStore in a few places, whereas the database engine expects a []string.

These two functions allow for more ergonomic conversions between NEX types and their primitives.
Using List[String] as an example, here's converting to primitives for the database:

getString := func(p *nextypes.String) string { return p.Value }

db.Query(`INSERT INTO...`,
    pq.Array(nextypes.TransformList(param.Tags, getString)),
)

Converting from primitives is nicer again, due to the NewString function already having the right signature:

var tags []string
db.Query(`SELECT ...`).Scan(pq.Array(&tags))

result.Tags = nextypes.NewListTransformed(tags, nextypes.NewString)

Ideally we would implement Scanner and Valuer so the database engine can work on our types directly (no conversions), but that would require adding database/sql/driver as a dependency which may or may not be acceptable. Let me know if I can do that though.

@jonbarrow
Copy link
Member

jonbarrow commented May 18, 2024

Ideally we would implement Scanner and Valuer so the database engine can work on our types directly (no conversions), but that would require adding database/sql/driver as a dependency which may or may not be acceptable. Let me know if I can do that though.

I was going to suggest this. We already do this for other types. Around a year ago I made https://github.com/PretendoNetwork/pq-extended which is a drop-in replacement for pq that adds support for more types, namely []uint8.

We could probably just add support for our types there, to keep the networking libraries "clean". But that would come at the cost of needing to manually handle each List type.

@DaniElectra what do you think? I'm not SUPER against adding the Scan/Value interface to our types in the networking libraries, but only if there's a lot of demand for it. It will require some refactoring to do, since we already use Value() and Value for our own needs

@DaniElectra
Copy link
Member

That sounds okay to me. We could add the Scan/Value interface here on the primitive types and then handle the lists on pq-extended

Though first we'd need a different name for our current use of Value. Maybe we can replace it with Real or something like that?

@jonbarrow
Copy link
Member

jonbarrow commented May 18, 2024

Honestly what if we revisit the idea of making primitives just type aliases, and even just not using pointers everywhere all the time?

In the rewrite PR I had said that this wasn't viable due to us needing to use pointer receivers, but I was slightly incorrect #42 (comment)

We CAN use type aliases here, while using pointer receivers, and conforming to RVType . We just have to make sure we call those methods on a pointer to the type:

package main

import (
	"fmt"
)

type RVType interface {
	ExtractFrom()
}

type UInt32 uint32

func (u32 *UInt32) ExtractFrom() {
	*u32 = UInt32(20)
}

func main() {
	var u32 UInt32 = 10

	fmt.Println(u32) // 10

	extract(&u32)

	fmt.Println(u32) // 20
}

func extract(t RVType) {
	t.ExtractFrom()
}

This also makes the types directly compatible with the built in types through type conversions:

package main

import (
	"fmt"
)

type RVType interface {
	ExtractFrom()
}

type UInt32 uint32

func (u32 *UInt32) ExtractFrom() {
	*u32 = UInt32(20)
}

func main() {
	var u32 UInt32 = 10

	fmt.Println(u32) // 10

	extract(&u32)

	fmt.Println(u32) // 20

	slice := make([]uint32, 0)

	slice = append(slice, uint32(u32))

	fmt.Println(slice) // [20]
}

func extract(t RVType) {
	t.ExtractFrom()
}

However, unlike in your comment here #42 (comment) we cannot, actually, use built in types in places like List.Append so long as those generics expect RVType. Which means:

list := List[UInt32]{}
list.Append(1)

Would NOT work here. We would still need to create a new instance of UInt32, and the List MUST take in pointers to the RVType still:

package main

import (
	"fmt"
)

type RVType interface {
	ExtractFrom()
}

type UInt32 uint32

func (u32 *UInt32) ExtractFrom() {
	*u32 = UInt32(20)
}

func NewUInt32(v int) *UInt32 {
	var u32 UInt32 = UInt32(v)
	return &u32
}

type List[T RVType] struct {
	real []T
	Type T
}

func (l *List[T]) Append(value T) {
	l.real = append(l.real, value)
}

func main() {
	var u32 UInt32 = 10

	fmt.Println(u32) // 10

	extract(&u32)

	fmt.Println(u32) // 20

	slice := make([]uint32, 0)

	slice = append(slice, uint32(u32))

	fmt.Println(slice) // [20]

	list := List[*UInt32]{}

	list.Append(&u32)

	fmt.Println(list) // {[0xc000012110] <nil>}

	list.Append(NewUInt32(1))

	fmt.Println(list) // {[0xc000012110 0xc00001212c] <nil>}
}

func extract(t RVType) {
	t.ExtractFrom()
}

@jonbarrow
Copy link
Member

That being said we still use Value on other types outside of primitives. For example DateTime and PID have Value() methods. Also several types in the protocols lib, like DataStoreKeyValue and SimpleSearchCondition have Value fields.

@jonbarrow jonbarrow mentioned this pull request May 22, 2024
4 tasks
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.

3 participants