[CXX-973] Rename bsoncxx::builder::stream::finalize to extract Created: 14/Jul/16  Updated: 08/Jan/24  Resolved: 03/Mar/17

Status: Closed
Project: C++ Driver
Component/s: API
Affects Version/s: None
Fix Version/s: None

Type: New Feature Priority: Major - P3
Reporter: David Golden Assignee: David Golden
Resolution: Won't Fix Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Epic Link: PM-329

 Description   

Because of the name, the finalize helper might easily be misunderstood as "finishing" a document being constructed, whereas it actually destructively steals the constructed document and returns it, leaving the builder with empty state.

Consider the following example:

using bsoncxx::builder::stream::document;
using bsoncxx::builder::stream::finalize;
 
document one;
one << "key" << "value";
std::cout << bsoncxx::to_json(one.view()) << std::endl;
 
document two;
two << "key" << "value" << finalize;
std::cout << bsoncxx::to_json(two.view()) << std::endl;

The result is:

{
    "key" : "value"
}
{
 
}

If we rename finalize to something more self-documenting (and alarming) like steal_document, the API would be harder to mistakenly misuse.

For example:

document two;
two << "key" << "value" << steal_document;

That is obviously a mistake. It also makes clear what assignment is doing:

document three;
auto doc = three << "key" << "value" << steal_document;



 Comments   
Comment by David Golden [ 03/Mar/17 ]

As we're de-emphasizing the stream builder, I'm closign this as won't fix.

Comment by Andrew Morrow (Inactive) [ 01/Feb/17 ]

I recommend just using in the new name as an alias of the old, or vice-versa, whatever the name may be, so you don't break existing code.

Comment by David Golden [ 12/Sep/16 ]

I'm open to an alternate name that is more self-documenting. Possibly extract or extract_document might be clearer, and we can link the docs to core::extract_document. That has some nice congruence between builders.

I also considered "release", but while the builder is releasing control, the actual return value is the new owner.

Comment by Mira Carey [ 12/Sep/16 ]

You're acting on the document stream builder and you're stealing the underlying byte buffer that the builder owns.

We're tunneling down to core::extract_document(), which calls impl::steal_document, which calls bson_destroy_with_steal, for what it's worth

Comment by Andrew Morrow (Inactive) [ 12/Sep/16 ]

I'm not entirely convinced that I want to make this change. I think the issue is that the concept is just tricky, and I don't think the proposed new name makes it any clearer. From what are you 'stealing'? You aren't even acting on the 'document' object when you put this type - you are acting on the 'type stack'.

Comment by Andrew Morrow (Inactive) [ 15/Jul/16 ]

If we rename it, we should using in the old name as an alias with the BSONCXX_DEPRECATED tag (I assume the deprecated atttribute works on aliases/typedefs), so that we don't break user code, and then update our examples and docs. mira.carey@mongodb.com - thoughts on whether we should find a better name, and, if so, what? I don't love 'steal', but I don't have a better suggestion at the moment.

Generated at Wed Feb 07 22:00:57 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.