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

Various Builder ::EndOf* functions should probably return mError, not *this #8309

Closed
bzbarsky-apple opened this issue Jul 12, 2021 · 2 comments · Fixed by #26910
Closed

Various Builder ::EndOf* functions should probably return mError, not *this #8309

bzbarsky-apple opened this issue Jul 12, 2021 · 2 comments · Fixed by #26910

Comments

@bzbarsky-apple
Copy link
Contributor

Problem

An example. ReadRequest::Builder & ReadRequest::Builder::EndOfReadRequest(). So consumers have to use it like so:

        request.EndOfReadRequest();
        SuccessOrExit(err = request.GetError());

But if we're at the end, what's the point of returning this? We don't in fact want to have any more calls adding more things to our TLV at this point!

Proposed Solution

If EndOfReadRequest returned mError, this would look like this:

SuccessOrExit(err = request.EndOfReadRequest());

which seems a lot more natural and harder to misuse.

@yunhanw-google

@yunhanw-google
Copy link
Contributor

Yep, agree, would update this next week, thanks

@stale
Copy link

stale bot commented Dec 3, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Dec 3, 2022
@bzbarsky-apple bzbarsky-apple removed the stale Stale issue or PR label Dec 9, 2022
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue May 27, 2023
Simplifies consumers and makes it clearer you're not supposed to use the object
any more after the EndOf... call.

Fixes project-chip#8309
bzbarsky-apple added a commit to bzbarsky-apple/connectedhomeip that referenced this issue May 27, 2023
Simplifies consumers and makes it clearer you're not supposed to use the object
any more after the EndOf... call.

Fixes project-chip#8309
andy31415 pushed a commit that referenced this issue May 29, 2023
…6910)

Simplifies consumers and makes it clearer you're not supposed to use the object
any more after the EndOf... call.

Fixes #8309
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants