Making 'Data' classes mutable

[ksuco...@gmail.com]

Since Protocol Buffer is designed primary as 'dumb objects' whose
primary purpose is the transfer of data over the wire / data-
persistency, should we relax the immutable data object vs. Builder
design constrain to make writing wrapper classes a whole lot easier?
I suggest the followings:

1. Make the default constructor public to allow instantiation of
data object (perhaps with default values in the
future)
2. Exposes setter methods to allow data objects to be mutable
3. Have protoc auto-generates a constructor that takes as inputs
all the mandatory fields. This makes it
clear what data must be set when coding using an IDE.

Since all the above basically makes the data class (as opposed to the
Builder class) mutable, a compile flag can be added to allow those who
needs to keep the data classes immutable keep the existing behavior
(the default behavior)... and to allow those who needs to have the
data class mutable from having to maintain their own special version
of protoc compiler.

[Kenton Varda]

The original version of Java protocol buffers (never released outside Google) used only mutable objects with no builders.  Most people who used it agree that moving to immutable objects was a very good design change.  When objects are mutable, it's easy for some piece of code to accidentally modify a message that it shouldn't be allowed to modify, and we saw bugs of this nature all the time.

[ksuco...@gmail.com]

I agree it is a good idea in general... However, if you need to wrap
up the generated code in some other model object (as is recommended in
the protocol buffer documentation) - you end up having to deal with
more complexity than is otherwise necessary. A protoc option gives
developers the freedom and flexibility to choose what's best for their
code.

We like protocol buffer over Thrift - as protocol buffer is a more
mutual product that also produces leaner encoded data. On the other
hand, we like Thrift because it gave us mutable objects, which makes
development easier for what we use ProtocolBuffer/Thrift for. We can
control data access through our own data model that wraps around
Thrift / ProtocolBuffer, so accidently changing data is not as much of
a concern for us.

On Aug 21, 11:30 am, "Kenton Varda" <ken...@google.com> wrote:
> The original version of Java protocol buffers (never released outside
> Google) used only mutable objects with no builders.  Most people who used it
> agree that moving to immutable objects was a very good design change.  When
> objects are mutable, it's easy for some piece of code to accidentally modify
> a message that it shouldn't be allowed to modify, and we saw bugs of this
> nature all the time.
>

[Kenton Varda]

If you really want a "mutable" message, you can always just use a Builder.  If there are reasons that doesn't work well, we should fix them.  I'd rather not massively change the design, even as an option.

[ksuco...@gmail.com]

Using builder is what was done initially. But it is not always a good
option for us....

Consider this simple example with a Car containing an Engine. If we
have two messages CaR and EnginE, and we write Wrapper around them -
Car and Engine - to add behaviors as follows:

class Car {
CaR wrapped;

public void accelerate();
public void decelerate();
...
public Engine getEngine() {
return new Engine(CaR.getEngine());
}
}

class Engine {
EnginE wrapped;

public void calibrate();
public void changeOil();
}

If CaR and EngineE are mutable objects with setters, to work on the
engine, we simply get the engine (i.e. the wrapped version) and call
the methods to calibrate and to change oil, for instance; the original
engine in the car is modified as intended - and multiple parties can
work on the engine at the same time without having to be concerned
about changes get lost.... Using builders, you'll need to modify a
new instance of the engine using Builder and set the new engine object
back onto the car object...

Perhaps I am missing something? But I think having setters makes the
design / object a whole lot easier to deal with -- in some cases.

On Aug 21, 5:00 pm, "Kenton Varda" <ken...@google.com> wrote:
> If you really want a "mutable" message, you can always just use a Builder.
> If there are reasons that doesn't work well, we should fix them. I'd
> rather not massively change the design, even as an option.
>

[Marc Gravell]

> Perhaps I am missing something? But I think having setters makes the
> design / object a whole lot easier to deal with -- in some cases.

I sympathise, and I can see the benefits of both approaches in
different scenarios; which is why for my C# implementation I opted for
the mutable object approach, safe in the knowledge that Jon was
working on the immutable/builder approach. This hopefully gives a
choice of either implementation for different scenarios (for .NET, at
least).

As an aside, the mutable approach doesn't preclude a specific object-
model manually implementing popsicle immutability if there is a need
for stability of an object in some cases...

Marc

[Torbjörn Gyllebring]

Actually protobuf-net and proto# both gives you the choice to do something like this: (Annotations omitted for brevity)

interface IFrozenYoghurt { public string Name { get; } }

class Yoghurt : IFrozenYoghurs{ public string Name { get; set; }

And thus giving you the option to pass around the immutable/frozen interface to clients when such semantics are needed.

[Marc Gravell]

> Actually protobuf-net and proto# both gives you the choice to do something
> like this: (Annotations omitted for brevity)

Well, that guarantees that you can't change it (short of casting), but
it doesn't guarantee that it doesn't get changed by something else -
but it is a start ;-p

For info, re "manually implementing popsicle immutability", I mean
something like:

class Foo {
// can we be edited?
public bool IsFrozen {get; private set;}
public void Freeze() {IsFrozen = true;}
protected void ThrowIfFrozen() {
if(IsFrozen) throw new InvalidOperationException("Cannot edit a
frozen object");
}

// a property that respects the freeze
private int bar
public int Bar {
get {return bar;}
set {ThrowIfFrozen(); bar = value;}
}
}

Actually, it would be pretty easy to get Post# to inject the frozen
tests rather than writing them all manually ;-p

Marc

[Torbjörn Gyllebring]

True, it's also fairly trivial to generate the popsicle immuatability classes given an "IFoo". 

And yes, it's not truly immutable anyone having a "Yoghurt" refrence would be able to change stuff, but's that a bit like protecting from machivelli if our dataaccess component only gives out refrences to the frozen interface we *should* be safe. Anyone starting to cast villy nilly just to break things is just as proabable to bring out the reflection hammer and and write directly to our private "bar" field.

[Marc Gravell]

True, true - but the point I was trying to make is that the caller
might not *have* to cast to mess things up...

RealObject foo = ...
DoWork((IRealObject)foo); // this method assumes immutable
foo.Bar = 123;
// oops, if "DoWork" works asynchronously we might just have ruined
things

But yes - an immutable interface addresses some of the issues.

Marc

[Kenton Varda]

I would be open to the idea of making each Builder possibly hold Builders for sub-messages instead of only holding immutable Message objects.  I think that would solve your problem.

[Kenton Varda]

So C# doesn't have "const", I take it?

[Marc Gravell]

> So C# doesn't have "const", I take it?

Not in terms of arguments / variables, no.

Marc

[ksuco...@gmail.com]

That sounds like an idea... So, continue with the Car / Engine
example used earlier, how do I get a builder for engine from the
builder for car? And, in this case, the purpose of the sub-builder is
to allow the modification of the engine object hold by car .... so I
suppose it will keep the object pass to it, instead of treating it as
a reference copy and make a cloned version? In this case, will
calling build() on the sub-builder return a clone version of the
original object or a cloned copy?

On Aug 22, 10:09 am, "Kenton Varda" <ken...@google.com> wrote:
> I would be open to the idea of making each Builder possibly hold Builders
> for sub-messages instead of only holding immutable Message objects. I think
> that would solve your problem.
>

[Kenton Varda]

Perhaps something like car.getEngineBuilder() would return a builder for the Engine.

Note that I'm not totally convinced that this is necessary.  Can't you just have independent builders for the Car and the Engine, and then put them together right before you actually serialize the protocol message?

[ksuco...@gmail.com]

I assume the car object in your example is a builder? Because
otherwise it would have been the same as if the setter(s) for the data
object for car is exposed directly. I still think optionally exposes
the setters for the data objects is a simpler and safe solution, The
architect of the system can always mandate the protocol buffer data
objects to be immutable - if doing so leads to a more stable system :)

Coming back to the Car / Engine example, if the intent is to wrap each
of te PB generated objects to allow adding more complex business logic
for each of the wrapped objects (other than the simple getters and
setters), it is more natural, and easier to maintain to have a 1:1
mapping between the wrapper object and the wrapped Protocol Buffer
object.... When you have lots of classes in your model, it becomes
very appealing very quickly to have a way to automatically generate
templates for the wrapper classes, or to have a generic adapter layer
using reflection for the case of Java...

On Aug 25, 10:53 am, "Kenton Varda" <ken...@google.com> wrote:
> Perhaps something like car.getEngineBuilder() would return a builder for the
> Engine.
> Note that I'm not totally convinced that this is necessary. Can't you just
> have independent builders for the Car and the Engine, and then put them
> together right before you actually serialize the protocol message?
>

[Kenton Varda]

On Tue, Aug 26, 2008 at 12:03 AM, <ksuco...@gmail.com> wrote:

I assume the car object in your example is a builder?

Right.

 

Because
otherwise it would have been the same as if the setter(s) for the data
object for car is exposed directly. I still think optionally exposes
the setters for the data objects is a simpler and safe solution, The
architect of the system can always mandate the protocol buffer data
objects to be immutable - if doing so leads to a more stable system :)

I really don't want to add an option that changes the fundamental design of the system.  The "architect" argument doesn't work when development is more decentralized, as in the open source world or inside Google.