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

Easy iteration and conversion of data rows. #528

Merged
merged 1 commit into from
Mar 26, 2022
Merged

Easy iteration and conversion of data rows. #528

merged 1 commit into from
Mar 26, 2022

Conversation

jtv
Copy link
Owner

@jtv jtv commented Jan 29, 2022

No description provided.

@jtv
Copy link
Owner Author

jtv commented Jan 30, 2022 via email

@KayEss
Copy link
Contributor

KayEss commented Jan 30, 2022

That contradicts the comment in the line above, and if it is meant to be a std::string, that appears to make the test identical to the one a bit further up.

@jtv
Copy link
Owner Author

jtv commented Jan 31, 2022

That contradicts the comment in the line above, and if it is meant to be a std::string, that appears to make the test identical to the one a bit further up.

I had made some changes to that comment since the version you looked at, so it's possible that the contradiction has disappeared.

The general idea is: there are no "regular" string conversions to string_view, but streaming queries and result sets do support them as special cases.

@jtv
Copy link
Owner Author

jtv commented Jan 31, 2022

I had made some changes to that comment since the version you looked at, so it's possible that the contradiction has disappeared.

Actually, I see now that the wrong comment is still there. It's possible that the correction is still sitting on my laptop unpushed. Fixing.

@jtv
Copy link
Owner Author

jtv commented Feb 5, 2022

So it works nicely. But I'm still not sure this is a good idea. I mean, how actually useful is it? It doesn't ever save you having to name the types, doesn't reduce type stutter — unless you already happen to have a callback in function form, or you just really prefer specifying the types there.

@KayEss
Copy link
Contributor

KayEss commented Feb 6, 2022

Personally I always feel that functional code looks better than imperative, so for me this looks like a nice API. To adapt from your test, I'd expect it to look like this in an application:

tx.exec("SELECT name, salary FROM employee ORDER BY name").for_each(
    [&](std::string_view name, int salary) {
        names.append(name);
        total += salary;
    });

This looks pretty clean to me, and an improvement over a raw for loop with explicit casts -- so I'd certainly use it.

@jtv
Copy link
Owner Author

jtv commented Feb 6, 2022

Personally I always feel that functional code looks better than imperative, so for me this looks like a nice API. To adapt from your test, I'd expect it to look like this in an application:

Thanks for the feedback @KayEss. I tried some alternate spellings using the existing API but nothing was significantly better than for_each, at least.

I think I'll continue this work then. But I feel compelled to do the same for streaming queries, which leaves me with some other template puzzles to solve.

Thanks @KayEss for helpful feedback.
@jtv jtv merged commit 9abe23a into master Mar 26, 2022
@jtv jtv deleted the for-each branch March 26, 2022 07:00
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.

2 participants