c# port need of getters and setters

[ghost]

Hi! just testing the new c# port code and trying to convert my protobuffer data to flatbuffers.

what i miss :

1. property getters instead of function calls (else no property reflection).
2. property setters instead of manual coding FlatBufferBuilder fbb = new FlatBufferBuilder(); Monster.startMonster(fbb); Monster.addHp(fbb, (short)80); int mon = Monster.endMonster(fbb); fbb.Finish(mon);

without the properties i need to write a wrapper class for each table or did i overlook the easy way to change values?....

[ghost]

Property getters may be doable, but setters currently less so, since the methods are static (they don't require a C# object to be created). I'll look into it.

[ghost]

any changes/progress in the c# code generator? =) http://www.gamedev.net/blog/49/entry-2259791-flatbuffers-in-net/

everything that makes the FB creation easier is welcome ...

[ghost]

All progress is right here on github, so no. The getters is something we want to change, and we'll get to it. Feel free to send us a PR is you're in a hurry.

[mormegil-cz]

The problem here is that each non-scalar field gets two read accessors: One parameterless, that could (should) be converted to property, but there is also the second accessor which receives an instance from outside as a performance optimization, and that (obviously) needs to be a method. However, in C#, you can’t have a property and a method with the same name. We could prefix the method, e.g. to GetYyy (MyType Yyy {get {…}}, MyType GetYyy(MyType obj) {…}), but this could lead to naming collisions (I could have Yyy and GetYyy in my schema). We could use some “ugly” prefixes like __GetYyy, but that is not really C# style (but could be acceptable for such members used only for performance optimization?). Or we could use properties only for scalar types where this problem does not occur, but that would be a bit strange and user unfriendly, IMHO.

But in fact, there are already possible naming collisions with methods like StartXxx and GetRootAsXxx. So we might as well just use GetYyy and be done with it. Objections?

[ghost]

Yes, that's a good summary of the issues. My solution was to simply not use properties, since having the two overloads have different names and be different things (properties vs methods) seemed inelegant. Also, the big benefit of properties is uniform reading & writing, whereas here we only read, so them not being properties seemed not quite as big a deal.

Additionally, writing to a FlatBuffer is completely separate from reading, so the two could never be combined in a single property. Having only half of that functionality in a property would maybe only add to the confusion.

I'd love to hear more opinions from C# programmers.

[mormegil-cz]

Take a look at my bcdd5941 which implements this change. I feel the properties are a better/more idiomatic way how to do that in C#, even with those downsides. (E.g., when I needed to debug my flatbuffer-powered communication, I tried to dump the incoming structure in LINQPad, only to learn there is nothing to be seen, as the structure has no properties, and I need to manually call each accessor method instead of just calling .Dump().)

Also, note that the process of reading and writing a flatbuffer are usually quite separate in your code, so the difference between x = MyObj.Prop; and ObjType.AddProp(x); is not really confusing, IMHO.

What do you think?

[ghost]

Ok, it seems that this is important for C#.. it is a breaking change, but unless anyone has objections we'll go ahead with it.

Two things that would be good to have as part of this commit:

• A paragraph in https://github.com/google/flatbuffers/blob/master/docs/source/JavaUsage.md showing how C# differs from Java.
• While we're breaking the API, maybe make the externally facing getters in FlatBufferBuilder.cs (Offset & DataBuffer) also into properties, to be consistent?

[mormegil-cz]

Right, and especially the Java-styled ByteBuffer.position() which I was almost shocked to find. ;-)

However, I am still a bit torn… The current result is quite a hodgepodge… Isn’t there a way to do that in a really simple, unified and idiomatic C# style?

[ghost]

Not sure about .position(), since the whole point of the C# ByteBuffer is to keep the interface the same as Java for easier code generation. I think only things that are user facing should be properties, internal implementation details are best kept as simple as possible.

[mormegil-cz]

Well, I am using .position() in my client code using FlatBuffers API, exactly according to the documentation:

The buffer is now ready to be transmitted. It is contained in the ByteBuffer which you can obtain from fbb.dataBuffer(). Importantly, the valid data does not start from offset 0 in this buffer, but from fbb.dataBuffer().position() (this is because the data was built backwards in memory). It ends at fbb.capacity().

And, AFAICT, the only alternative to using .position() would be to make a copy of the buffer with .SizedByteArray(), which is quite unnecessary when e.g. writing the buffer to a stream. Or, is there a better way (in which case, the documentation should be updated)?

[mormegil-cz]

I made a few more unificating changes, so that all accessor methods start are named GetXXX, and I changed the API classes to use properties per the above comments. The end result (and its usage) now looks almost acceptably, I’d say… :-)

But an opinion from another C# programmer would be most welcome. What do you think, @XBog?

[ghost]

Sorry, you're right on position(). New changes look good to me.

Could you also update https://github.com/google/flatbuffers/blob/master/docs/source/JavaUsage.md showing how C# is different where appropriate?

[ghost]

Feel free to make a PR when ready.

[ghost]

hey momegil-cz! nice work. I had a bit more drastical changes in mind :) But as gwvo already said : setters currently less so, since the methods are static (they don't require a C# object to be created) The perfect generated code for me to use, would be: Monster monster = new Monster(); monster.Mana = 100; .... byte[] example = monster.Bytes;

But that just seems not (so easy) "possible".

[ghost]

XBog: that inferface is possible, but it would be very inefficient compared to what we currently have.

[ghost]

efficient > usability. But optional? As i said, "for me" it would be perfect, because i`m using c# only for flatbuffer bytedata generation that don't change at runtime. For that i'm currently writing wrapper classes around the flatbuffer classes, to be able to use the normal "automatic" c# properties reflection/serialization. For the dynamic data i'll use c++ . So yes, if i would need efficiency, i would stay with the current solution. it's just my laziness :)

[mormegil-cz]

@gwvo: I think the code (including documentation) are ready in my branch, do I need to squash it for a PR, or is that unnecessary, given you cherry-pick it to Gerrit, anyway?

[mormegil-cz]

I have squashed the commits and created the PR, see above.

[ghost]

This is now in master: https://github.com/google/flatbuffers/commit/0ee1b99c5d38636c2ab8eb8e05dcd42b328c0cca Thanks Petr.