Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

glib: Add new object constructor for constructing an object with default property values #910

Merged
merged 2 commits into from Jan 21, 2023

Conversation

sdroege
Copy link
Member

@sdroege sdroege commented Jan 21, 2023

Nicer than writing Object::new(&[]).

Also optimize the object builder a bit.

@pbor
Copy link
Contributor

pbor commented Jan 21, 2023

Is this really that common that we need to optimize for it? Usually this is wrapped into a new() method for the concrete object.

Maybe we should impl Default and have it call Object::new(T::static_type(), &[])?

new_default_with_type seems even less common and if one is writing some kind of factory object they can stick to passing in &[]

@sdroege
Copy link
Member Author

sdroege commented Jan 21, 2023

Is this really that common that we need to optimize for it? Usually this is wrapped into a new() method for the concrete object.

All the Default impls are calling new(&[]) currently, and I know that I'm having to write this quite often for subclasses. So at least for me it's going to save some annoying typing :)

Maybe we should impl Default and have it call Object::new(T::static_type(), &[])?

This would be called from the impl Default, yes.

Previously we were copying each parameter value potentially twice, now
it is only going to be copied inside C if at all.

Also allocate the array of properties on the stack if only a few
properties are used.
@sdroege
Copy link
Member Author

sdroege commented Jan 21, 2023

Also note that I'm working on deprecating ::new() and ::with_type() and ::with_values() as those all unnecessarily copy the property values twice.

@pbor
Copy link
Contributor

pbor commented Jan 21, 2023

Ah ok then, if the redundancy is removed the other way round then ok

@pbor
Copy link
Contributor

pbor commented Jan 21, 2023

Initable and AsyncInitable will need the same updates for consistency

@sdroege
Copy link
Member Author

sdroege commented Jan 21, 2023

Initable and AsyncInitable will need the same updates for consistency

Yeah they should also get builders. That API became quite bitrotten already...

@sdroege sdroege added this to the 0.17.0 milestone Jan 21, 2023
@sdroege
Copy link
Member Author

sdroege commented Jan 21, 2023

As discussed, names are as good as it can be for this release and for the release afterwards (after having deprecated new() for this release), we can switch over from new_default() to new(). There's also an issue so we don't forget.

I'll finish the (async) initable changes and then merge this.

…ult property values

And also keep the `gio::Initable` / `gio::AsyncInitable` API in sync,
i.e. add a builder and the new contructors there too.
@sdroege sdroege merged commit b67a499 into gtk-rs:master Jan 21, 2023
@sdroege sdroege deleted the simpler-object-construction branch January 21, 2023 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants