MelGrubb / BuilderGenerator

A source-generator-based implementation of the Builder pattern
https://melgrubb.github.io/BuilderGenerator/
MIT License
38 stars 8 forks source link

Add support for Protobuf RepeatedField collections #31

Open leus opened 1 year ago

leus commented 1 year ago

Protobuf generated entities seem to work well with BuilderGenerator but collections don't get generated. Protobuf collections are of type RepeatedField which are read-only properties that implement IEnumerable.

leus commented 1 year ago

Looking into the code, one could argue that it's worth adding support for read-only collections leveraging something like Add() or AddRange() in the implementation template.

MelGrubb commented 1 year ago

Hmm. The trouble with IEnumerable is that it doesn't have Add or AddRange. You need ICollection to get those. I took a quick glance, though, and it looks like it inherits from IList, which would have those. The more general problem is knowing exactly which type of collection to generate. I'll need to think about that.

leus commented 1 year ago

Yeah, I think you are right supporting IEnumerable is tricky (enumerator functions, backed fields, etc.) but ICollection shouldn't be hard (just use AddRange and forget about it).

MelGrubb commented 1 year ago

Expanding collection support is on my list, but I'm not sure when it'll get done. I think it will require more reflection than what the code is doing now. It will need to be intelligent about whether to just clear the collection or to replace it completely, depending on the properties of the collection itself. What if the class has a settable collection and it's empty. If you've told the builder to put something in the collection, it will have to be smart enough to set it to an appropriate type and then fill it. I like that the library is useful to others, and I'm starting to suspect that it's going to grow into a much larger project than it is. As such, I'm trying to tackle some optimizations first before moving forward. I'll keep this here so that I don't lose track, though.