>
> On Fri, May 25, 2018 at 07:33:56AM -0400, Frediano Ziglio wrote:
> > > +static gboolean playback_channel_client_initable_init(GInitable
> > > *initable,
> > > + GCancellable
> > > *cancellable,
> > > + GError **error)
> > > +{
> > > + gboolean success;
> > > + RedClient *red_client =
> > > red_channel_client_get_client(RED_CHANNEL_CLIENT(initable));
> > > + SndChannelClient *scc = SND_CHANNEL_CLIENT(initable);
> > > + RedChannel *red_channel =
> > > red_channel_client_get_channel(RED_CHANNEL_CLIENT(initable));
> > > + SndChannel *channel = SND_CHANNEL(red_channel);
> > > +
> > > + success =
> > > playback_channel_client_parent_initable_iface->init(initable,
> > > cancellable, error);
> > > + if (!success) {
> >
> > I think is short and easier to read doing the if directly.
>
> Probably a matter of preference, I prefer to have functions with side
> effects separate from their if (success) test
> > > + iface->init = record_channel_client_initable_init;
> > > +}
> > > +
> > > +
> > > static void
> > > record_channel_client_class_init(RecordChannelClientClass *klass)
> > > {
> > > GObjectClass *object_class = G_OBJECT_CLASS(klass);
> > > - object_class->constructed = record_channel_client_constructed;
> > > object_class->finalize = record_channel_client_finalize;
> > > }
> > >
> >
> > Is it only me or che code looks a lot duplicated?
>
> A lot duplicated? It's a bit messy between ::init(), ::constructed() and
> ::initable_init(), but if I did not mess things up, they should be doing
> different things (ie no actual code duplication).
>
Forgot to add OT, looks like the 2 _init/_constructed are mostly
doing the same thing. But is not a regression.
> > Reusing constructed function instead of removing and create new
> > function in different places won't reduce the diff?
>
> RedChannelClient::initable_init() is needed as this is the only place
> where you can handle and report init failures, which is why some code is
> there. Since snd_send() needs to run after the code in
> RedChannelClient::initable_init(), that code is moved there.
>
> Or where you pointing out a different issue?
>
Was more a patch style thing.
The new _init are doing more or less the same thing as the old
_constructed but as the new functions are in different lines
in the final code the diff results huge. Probably if you put
the new _init where the old _constructed were the diff will be
smaller and is easier to understand the real change.
> Christophe
>
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel