Comments (12)
I'm afraid I don't have any time to implement this in the short/medium term. We
*could* potentially add some sort of optional interface, implemented via
partial classes on the message and its builder, but it's not terribly ideal.
Original comment by jonathan.skeet
on 15 May 2012 at 5:27
from protobuf-csharp-port.
I was going to go with locally subclassing the JsonFormatWriter/Reader and add
whatever I need to there, but was looking for some feedback on what would be
ideal.
Original comment by [email protected]
on 15 May 2012 at 5:47
from protobuf-csharp-port.
I tried with the interface, and I didn't like it that much. Instead I tried
again with a JsonConverter class, like Newtonsoft.Json (Json.Net) has and it
seemed to work pretty well.
I did it by extending JsonFormatReader and JsonFormatWriter in my local project
and adding the converter stuff there. The trouble is that JsonFormatReader and
JsonFormatWriter have key extension points locked down, like private inheriting
concrete classes that make it impossible to extend without modification.
The first change to protobuf-csharp should be to just open those classes and
move JsonTextWriter and JsonStreamWriter into a separate class so they don't
inherit from JsonFormatWriter. I'll put together a patch for that.
Original comment by [email protected]
on 16 May 2012 at 8:24
from protobuf-csharp-port.
I'm certainly happy to look at a patch, but I personally generally prefer
composition over inheritance in various places. I think I'll have to have a
look at various options before putting anything into the code base.
Of course, you'd be welcome to use your own fork in the meantime :)
Original comment by jonathan.skeet
on 16 May 2012 at 8:51
from protobuf-csharp-port.
Can you please review the two commits I made? I separated them, so you can
decide how far you want to go with this.
The first commit, eb6d0ea94e05, enables the extensability needed, some of it is
required even if the JsonConverter functionality is built in.
http://code.google.com/r/nathan-protobuf-csharp-port/source/detail?r=eb6d0ea94e0
58fdd31a1524df449f172e99f995c&name=issue-44
The second commit, 9d5c01bc5e20, builds on that and builds JsonConverter
functionality in.
http://code.google.com/r/nathan-protobuf-csharp-port/source/detail?r=9d5c01bc5e2
00823c084c1773db7c3a49007d6b5&name=issue-44
Thanks
Original comment by [email protected]
on 17 May 2012 at 7:52
from protobuf-csharp-port.
Nathan,
Not sure if we want to expose the inner workings of these classes as they could
change drastically in the future. This was the reason they are essentially
sealed classes (you can't derive from them).
One viable option would be to replicate the entire JsonReader/Writer classes in
your local project. You should have no issues deriving from the
AbstractTextReader/Writer classes to perform whatever magic you need. This
really isn't that much code and will isolate from changes to those classes in
the future.
The other option of course is to implement the ICodedInput/OutputStream
interfaces and aggregate the behavior of the json/xml reader/writer. Mostly
you will pass-through the calls directly to the underlying implementation. The
special casing should be able to be handled in WriteMessage(...) for the
ICodedOutputStream, and ReadMessage(...) for ICodedInputStream. I think this
option would prove the most durable against future changes if you can make it
work for you.
If you what to pursue the later option here let me know, I'll lend you what aid
I can.
Original comment by [email protected]
on 18 May 2012 at 12:27
from protobuf-csharp-port.
I agree with Roger - I'd rather not start making the guts internal. Once folks
(such as yourself) have started taking dependencies on the implementation
details, it's going to make it much harder to change them later.
Given that Roger's worked much more on the JSON than I have, I'd suggest you
take him up on his offer of help ;)
Original comment by jonathan.skeet
on 18 May 2012 at 7:42
from protobuf-csharp-port.
Nathan,
The attached zip file contains three sources files than can be added to the
"ProtocolBuffers.Test" project. Two of these, AggregateInputStream.cs and
AggregateOutputStream.cs are simple aggregations of the reader/writer
interfaces. The last file, TestCustomReaderWriter.cs, contains derivations
that customize the behavior of ReadMessage and WriteMessage to perform the same
behavior as your example unit test. The test also demonstrates that this
technique can be applied to any format.
If Jon agrees, I would be very willing to include the AggregateXxxxStream
objects as these are quite difficult to produce due to the verbosity of the
interfaces. Additionally having these defined in the library would isolate
implementations from minor interface changes.
Anyway, take a look at the CustomMessageReader/CustomMessageWriter
implementations for your example and let me know what you think.
Original comment by [email protected]
on 22 May 2012 at 12:21
Attachments:
from protobuf-csharp-port.
I think I'd called it DelegatingMessageReader (etc) instead of Aggregate - it's
not aggregating multiple streams, after all. But the overall approach seems
sound, yes. I think such code will always have to be written pretty carefully,
mind you.
I'd also prefer the InnerStream to be a property instead of a protected field,
but that's a minor detail :)
Original comment by jonathan.skeet
on 23 May 2012 at 7:20
from protobuf-csharp-port.
@Jon, No problem on the rename and use of property.
@Nathan, Have you had the chance to try this out and see if it works for you?
Original comment by [email protected]
on 29 May 2012 at 3:55
from protobuf-csharp-port.
Original comment by [email protected]
on 11 Oct 2012 at 12:07
- Changed state: Accepted
from protobuf-csharp-port.
Original comment by [email protected]
on 11 Oct 2012 at 12:07
from protobuf-csharp-port.
Related Issues (20)
- "GenericArguments[0], 'SystemMessageTest', on 'Google.ProtocolBuffers.IBuilderLite`2[TMessage,TBuilder]' violates the constraint of type parameter 'TMessage'." HOT 3
- .NET Core support HOT 2
- Problem with required enum field with default value HOT 3
- Problem with Protobuf on IOS HOT 2
- fig
- Build protobuf-csharp-port with VS2015 HOT 1
- repeated member of message with suffix "List" cause compile error HOT 1
- How to access GetExtension in c# like c++ HOT 3
- out of memory HOT 5
- No support for enums 'allow_alias' feature HOT 1
- src/ProtoBench/Program.cs(254,65): error CS0103: The name `JsonReaderWriterFactory' does not exist in the current context HOT 1
- Patch for /src/ProtocolBuffers/Properties/AssemblyInfo.cs HOT 10
- Unknown fields are not serialized to XML or JSON formats HOT 6
- csharp_options.proto breaks python HOT 4
- Deserialization ignores a nonempty field HOT 6
- Support new oneof feature HOT 11
- Custom Options with more than one required field can't be used HOT 8
- CodedInputStream.ReadEnum performance HOT 3
- How can I use csharp-port 2.4 with mono in IOS? It always got a Jit error message. HOT 9
- Builder.Clear() does not clear enum fields HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from protobuf-csharp-port.