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

Feature fields getters and setters should accept field indexes #500

Open
lnicola opened this issue Dec 20, 2023 · 3 comments
Open

Feature fields getters and setters should accept field indexes #500

lnicola opened this issue Dec 20, 2023 · 3 comments

Comments

@lnicola
Copy link
Member

lnicola commented Dec 20, 2023

I was recently looking at a small program that spent 30% of its CPU time in strcasecmp.

To be honest, I wish they'd only take the field index and never the name, but that's might really upset some users.

@ChristianBeilschmidt
Copy link
Contributor

Well, a resolution from name to index is a convenience thing, I guess. There could be two separate methods or one that accepts both. But you definitely need the opportunity to use the names.

So, what is your suggestion here?

@lnicola
Copy link
Member Author

lnicola commented Jan 30, 2024

So we can add fn set_field_by_index(&self, idx: usize, value: &FieldValue), fn set_field_integer_by_index(&self, idx: usize, value: i32) and about 9 more.

But I'm worried that users will just do:

for feature in layer.features() {
    feature.set_field_integer("foo", 20);
}

instead of:

let foo_col = layer.field_index("foo")?;
for feature in layer.features() {
    feature.set_field_integer_by_index(foo_col, 20);
}

just because it's easier to find.

@ChristianBeilschmidt
Copy link
Contributor

Yeah, either it would be …by_index and …by_name separately or you just have to keep strongly typing it to size 😄 .

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

No branches or pull requests

2 participants