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).
> 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?
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel