microsoft / bond

Bond is a cross-platform framework for working with schematized data. It supports cross-language de/serialization and powerful generic mechanisms for efficiently manipulating data. Bond is broadly used at Microsoft in high scale services.
MIT License
2.61k stars 322 forks source link

Support Bond IDL mapping to C# nested classes #357

Open aschmahmann opened 7 years ago

aschmahmann commented 7 years ago

I currently have some C# classes that I'm attempting to modify for use with Bond.

Some of these classes are nested, e.g.:

public class BunchOfClasses {
       public class SerializableSubclass {
           public int numberField;
           public void DoStuff() { 
             //things
           }
      }
}

It would be useful to me to allow creation of Bond IDL that will generate

public partial class BunchOfClasses{
       public partial class SerializableSubClass {
               public int numberField; //Or as properties, or any of the other IDL field generating options
       }
}

then all I have to do to for the conversion is delete the old field and put partial modifiers on the classes. Currently there doesn't seem to be a way of describing nested classes so the only options are manual code generation instead of the IDL or extracting my nested classes out of their parent class. It's possible I may pull the nested classes from their parent anyway, but this seems like an issue people will eventually run into.

chwarr commented 7 years ago

Do your inner classes access internal/protected/private members of BunchOfClasses? (e.g., you pass an instance of BunchOfClasses to the nested instances when you construct them) If not, then I'd try converting to nested namespaces. That might be an easier path forward. If they do access private things, converting to namespace won't help.

That being said, I don't think that supporting structs defined inside of structs in a good long term direction. Not all of the languages we're looking at targeting in the future support defining structs/classes/objects inside of one another.

Thoughts?

aschmahmann commented 7 years ago

As I mentioned above, it's possible for my case that pulling the classes into a separate namespace + a little refactoring will be enough to make this work. I definitely agree that having language specific features in the IDL is not great.

Perhaps a way to do this is basically by giving the language specific code generators more flags. For example, if I could name my IDL struct BunchOfClasses.SerializableSubClass then I could flag the c# generator to do what I mentioned above. However, for another language BunchOfClasses.SerializableSubClass could be interpreted as BunchOfClassesSerializableSubClass or BunchOfClasses or SerializableSubClass.

There are likely better ways to do this then I've suggested, and perhaps this feature is a little too language specific to be generally useful, but for those that have a harder time extracting nested classes into new namespaces it might be worthwhile.

chwarr commented 7 years ago

We're planning to start a large push on more robust target-language specific settings. I'm going to park this issue for now, and see whether something falls out of the forthcoming design work. Thanks for the ideas.

aschmahmann commented 7 years ago

Sounds good, if I come across other issues like this in my code conversion I'll post them so you guys have that info available.