evgenekov / protobuf-csharp-port

Automatically exported from code.google.com/p/protobuf-csharp-port
Other
0 stars 0 forks source link

Feature: add option to avoid nested messages to be under "Types" class. #75

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Please provide any additional information below.

Consider the following .proto file:

message OutterMsg {
  message InnerMsg {}
  optional InnerMsg inner_msg = 1;
}

Currently, produced code looks like this:

class OutterMsg {
  class Types {
    class InnerMsg {...}
  }
  Types.InnerMsg InnerMsg;
}

Which means that to declare a builder for InnerMsg, one needs to type:

  OutterMsg.Types.InnerMsg msg;

That's rather long.

It's known that "Types" nested class is used to avoid clashing sub-message and 
Property names.

Another way to avoid such problem is to prefix sub-message names. For instance, 
a leading underscore char should suffice for 99% of the cases. Produced code 
would look like the following:

class OutterMsg {
  class _InnerMsg {...}
  _InnerMsg InnerMsg;
}

Declaration would look like:

  OutterMsg._InnerMsg msg;

A per-message (and perhaps a per-proto-file) option string 
csharp_nesting_prefix should work just fine. Proto file would look like the 
following:

message OutterMsg {
  option (csharp_nesting_prefix) = "_";

  message InnerMsg {}
  optional InnerMsg inner_msg = 1;
}

Original issue reported on code.google.com by igorga...@gmail.com on 21 Feb 2014 at 11:22

GoogleCodeExporter commented 9 years ago
Hmm. This is possible, but I'm not sure I like it.

Note that a using directive of "using InnerMsg = 
Foo.Bar.OuterMsg.Types.InnerMsg" will let you use just InnerMsg in code where 
it starts getting ugly... I think that would generally be a cleaner approach. 
Again, let me think about this more. It's possible that it would be simpler to 
allow you to remove the nesting (that would be easy) but not change the class 
name - instead notice when there's a collision and require that the property 
name is changed.

Original comment by jonathan.skeet on 5 Apr 2014 at 3:25

GoogleCodeExporter commented 9 years ago
Hey Jon, any thoughts about this. The "using" approach is kind of annoying. Say 
you have:

message SomeMessage {
  enum Status {}
}

You start seeing people type:

  SomeMessage.Types.Status

Others use the "using" approach but since "Status" is a rather generic name, 
they do:

  using SomeMessageStatus = SomeMessage.Types.Status;

The fact now there are two names for the same enum is troublesome.

Prefix approach may sounds bad. A more clean approach would be a bool selecting 
whether "Types" should actually exist or not. Typing "SomeMessage.Status" is 
just way better.

Original comment by igorga...@gmail.com on 28 May 2014 at 2:59

GoogleCodeExporter commented 9 years ago
Yes, "SomeMessage.Status" would be nicer - but *usually* there's a nested type 
because there's a field of that type as well. For example:

message SomeMessage {
  enum Status {}

  optional Status status = 1;
}

That's the common case which cases the issue. I'll think about it - I'm 
uncomfortable with throwing options at every problem, as they mean that every 
codebase that uses protocol buffers ends up looking different. But I'll keep 
thinking...

Original comment by jonathan.skeet on 29 May 2014 at 6:09

GoogleCodeExporter commented 9 years ago
Leaving this open as it may well be a C#-specific thing to think about in the 
3.x timeframe. The additional Types really *is* annoying :(

Original comment by jonathan.skeet on 15 Feb 2015 at 5:14