mongocxx bsoncxx new c++11 driver: Basic Question: try-catch / fin_one_..() / owning and non owning

[javantamt]

Hey =),

there is a fiunction find_one_and_update().
This function can throw a exception like: mongocxx::write_exception.
So, there have to be a try catch around it.
The function is returning a bsoncxx::document::value.
If I leave the try-catch-block this value is destroyed.

Problem:

    try{
        valResult = MyMongo.collTasks.find_one_and_update(viewFind,viewUpdate,TheOptions);
    }catch(mongocxx::write_exception &e){
        std::cout << e.waht() << std::endl;
    }
    // here I'm going to use it, but I'm out of scope.

The work around I tried was:

    bsoncxx::document::value valResult; // but this line doesn't work.
    try{
        auto tmp = MyMongo.collTasks.find_one_and_update(viewFind,viewUpdate,TheOptions);
        valResult = std::move(tmp);
    }catch(mongocxx::write_exception &e){
        std::cout << e.waht() << std::endl;
    }

Im going to avoid a huge try-scope.
And I'm going to pass the value later to a calling function via reference.
But passing the value as a reference causing same problems, because I can't build a empty bson::document::value in the calling function.

myfunction(bsoncxx::document::value * pTheResult){
   
    ...
    * pTheResult = std::move( valResult );
}

The aim is to call a function, which is doing a lot of stuff and returning just some documents from database via reference.
Maybe there is an other way in doing this and my chosen path is to complicated.

This scope thing happens to me in sveral places in my code and I'm not happy with the solutions or workarrounds I explored so fare.

javantamt

[Andrew Morrow]

Hi -

Thanks for sending this. It is an interesting set of notes.

I think the fundamental issue you are encountering is that bsoncxx::document::value objects are not default constructible. If they were, your second code example would work just fine.

There is a reason though that bsoncxx::document::value does not offer a default constructor. The document::value class is intended to model the concept of "an owned region of bytes containing BSON which can expose a view". If we permitted default construction of value, we would need to do one of the following:

- Allow it to point to nothing (nullptr), which would break the invariant that it owns a region of memory, as well as the invariant that it contains BSON that you can view.

- Allocate 5 bytes on the heap to hold an empty BSON document. That would retain the invariants, but it would mean that simply declaring a document::value on the stack would involve a heap allocation. In most cases, that heap allocation would be wasted, as in your example where you almost always move a new BSON value in.

So, neither of these is particularly appealing.

However, I think there is an easy way out, which is to use either bsoncxx::stdx::optional or bsoncxx::view_or_value to hold the result. I'd lean towards stdx::optional, since bsoncxx::view_or_value is really intended only for parameter passing. I think it also would be easy to get in trouble with the latter type - it has tricky semantics.

So, with stdx::optional and taking your second example, you would get:

    stdx::optional<document::value> valResult;

    try{

        auto tmp = MyMongo.collTasks.find_one_and_update(viewFind,viewUpdate,TheOptions);

        valResult = std::move(tmp);

    } catch(const mongocxx::write_exception &e){

        std::cout << e.what() << std::endl;

    }

    if (valResult) {

        // Use valResult

    }

There are a few things to note here though. First, because you are falling through the catch block, you can't know whether valResult is an engaged optional or not, so you must have the if statement. In real world code where presumably you did not continue after the exception, you would know that valResult was engaged, so you could elide the conditional.

That said, using the optional here is unsatisfying.

Your third example is also interesting. My suggested work-around here would be to not use out parameters from your functions, and simply return the document::value:

 

--
You received this message because you are subscribed to the Google Groups "mongodb-user"
group.
 
For other MongoDB technical support options, see: https://docs.mongodb.org/manual/support/
---
You received this message because you are subscribed to the Google Groups "mongodb-user" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mongodb-user...@googlegroups.com.
To post to this group, send email to mongod...@googlegroups.com.
Visit this group at https://groups.google.com/group/mongodb-user.
To view this discussion on the web visit https://groups.google.com/d/msgid/mongodb-user/88053617-2285-4a13-8feb-1a2d9c3885b5%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Andrew Morrow]

Apologies, I hit send to early. I was going to show a re-written version of your function, but I think the idea is clear. However, I would argue that mandating that functions emitting document::value must place the value in the return slot is also suboptimal, and the lack of default constructibility will propagate virally to any enclosing types. Again stdx::optional would work, but it is inelegant.

I think the issues you have raised are important enough that I've filled https://jira.mongodb.org/browse/CXX-939 so that the C++ driver team can investigate.

Thanks,

Andrew

[javantamt]

Hey Andrew,

thank you for this detailed informations about concept behind the driver. And your solutions.
In the meanwhile I had some more ideas which are not elegant too. For example having two times "return MyValue;" on for the succesful way, one for the other. But a function should have only one exit.
For don't loosing the scope I have packed the value into a vector which is in the upper scope...
But a vector for only one element...

thx for CXX-939,

javantamt

[javantamt]

Working!

But what is better?
2 , 3 or 4?^^

void TryingStdxOptional() {
  // upper scope
  bsoncxx::stdx::optional<bsoncxx::document::value> MyOptVal;

  {
    // generating
    MyOptVal = document { } << "Key" << "val" << finalize;
  }
  {
    // 1
    // this is a example: Test the MyOptVal if it has content in RW
    bsoncxx::document::value theRealVal = MyOptVal.value();
    std::cout << "My document:\n" << bsoncxx::to_json(theRealVal) << std::endl;
  }
  {
    // 2
    // generated once in the beginning of the new scope.
    bsoncxx::document::value theRealVal = MyOptVal.value();
    // doing some more stuff with the value
    std::cout << "\nMy document ( again in new scope ):\n" << bsoncxx::to_json(theRealVal) << std::endl;
  }
  {
    // 3
    // everytime extraced when used.
    std::cout << "\nMy document ( the more sheeper way? ):\n" << bsoncxx::to_json(MyOptVal.value()) << std::endl;
  }
}

[li ning]

I think the problem is the methods supplied by collection class are inconsistent when report an error. There are 2 ways to report error: throw exception or return an error code. But collection class use them both, I don't know why it needs both of them, especially it already create some result type like delete_result.
It is better to return an find_and_update result type rather than throw exception.
Further, there is no need wrap result type by optional. This is odd, for example:
optional<delete_result> res = collection.delete({something});
if(!res)
{

    //what is this mean? It doesn't make any sense.

}
I notice you post an issue, hope the maintainer can read this.


在 2016年6月19日星期日 UTC+8下午9:45:04，acm写道：

[Andrew Morrow]

Hi -

Actually, we have discussed this before, in another thread. You appear to disagree with our choice to report errors via exceptions. That is fine, and you are not the only one to dislike exceptions. You are welcome to wrap our API, catch exceptions, and report them via std::error_code or a similar convention in your code. Had we written an API that returned error codes instead of exceptions, someone preferring exceptions would have found that choice distasteful as well.

To your question about why some of these functions both return a result and throw an exception, the answer is that these methods do in fact return something - an object from the mongocxx::result namespace that describes the result of the database operation, assuming that it did not fail and raise an exception. This in fact is one of the things that pushed us towards using exceptions: had we not done so, the return slot would have been consumed by the error reporting object, and we would have been required to either:

- Return 'fat' results that contained both successful operation results and error codes, only some subset of which fields would have been valid or meaningful for any one operation.

- Return a std::tuple, std::pair, or something more like the proposed std::expected or Variant/Either type containing both/either the result and/or the error.

- Use out parameters for the database results.

None of these seemed like a better choice than simply using exceptions, which have the added benefit that users are forced to deal with them due to the non-local flow of control they impose. Error returns can be ignored.

Finally, you ask why the return value is an optional object. We return an optional object because in some write concern modes (notable w:0 a.k.a unacknowledged mode) the database does not return a result. By returning an optional result, we can model that situation. In practice, the C++11 driver does not currently do this, because of a bug in the underlying C driver (see https://jira.mongodb.org/browse/CXX-894). When that bug is fixed, the C++11 driver will sometimes return disengaged optional results.

I'm happy to provide more details on any of the above points if you are curious.

Thanks,

Andrew

To view this discussion on the web visit https://groups.google.com/d/msgid/mongodb-user/09be518a-d95e-40cf-a7b6-2651e04f7514%40googlegroups.com.

[Andrew Morrow]

On Sun, Jun 19, 2016 at 8:05 PM, javantamt <java...@gmail.com> wrote:

Working!

But what is better?
2 , 3 or 4?^^

void TryingStdxOptional() {
  // upper scope
  bsoncxx::stdx::optional<bsoncxx::document::value> MyOptVal;

  {
    // generating
    MyOptVal = document { } << "Key" << "val" << finalize;
  }
  {
    // 1
    // this is a example: Test the MyOptVal if it has content in RW
    bsoncxx::document::value theRealVal = MyOptVal.value();

You just made a copy! That's ok, but it isn't free. Try theRealVal = std::move(MyOptVal.value());

 

    std::cout << "My document:\n" << bsoncxx::to_json(theRealVal) << std::endl;
  }
  {
    // 2
    // generated once in the beginning of the new scope.
    bsoncxx::document::value theRealVal = MyOptVal.value();
    // doing some more stuff with the value
    std::cout << "\nMy document ( again in new scope ):\n" << bsoncxx::to_json(theRealVal) << std::endl;
  }

You are also making a copy here when you declare theRealVal. No copy occurs when passing theRealVal to the to_json method, as that method takes a view, and the value is implicitly convertible.

I don't think there is really any difference between this and '1' above.

 

  {
    // 3
    // everytime extraced when used.
    std::cout << "\nMy document ( the more sheeper way? ):\n" << bsoncxx::to_json(MyOptVal.value()) << std::endl;
  }
}

This will not make a copy. Instead, the value& returned by optional::value is being passed

Overall, I don't think there is much difference between these cases, really. More important to focus on when you are moving or copying values (try not to copy them), and when passing those values (like to to_json) will automatically convert to a view (most places can take views, passing values to those functions does not result in a copy, which also has implications for how you manage lifetimes).

Thanks,

Andrew

[li ning]

Thanks for your explanation. I have not say I dislike exception, I just feel current mongocxx report error way is a bit redundant from user's perspective. For example:

try{

optional<result::insert_one> res = col.insert_one({...});

if(res->result().inserted_count() == 0)

    // 1 impossible?

}

catch(mongocxx::write_exception& e)

{

    // 2 there has same _id doc already

}

For user, those 2 comments basically are same thing even they are totally different situation for driver or db. User's business logic are splitted to 2 block, sometimes it is inconvenient to organize the code as this topic describe.

If add an error code member in each result type like:

class insert_one_wrapper {

int error_code;

std::optional<insert_one> result;

};

result::insert_one_wrapper res = col.insert_one({...});

if(res.error_code)

    // log and return

const result::bulk_write& r = *(res.result);

if(r.inserted_count() == 0)

    // there has same _id doc already

Now the duplicated _id insert was not considered as an error, if there is a error that should means an error which user can no nothing, it must cancel this operation. like std::bad_alloc

User only need check the wrapped result rather than needs care exception and result both.

May be some detail is improper, but just show the way

在 2016年6月20日星期一 UTC+8下午9:52:34，acm写道：

[Andrew Morrow]

Hi -

It is fully expected that users of the C++11 driver will build their own abstractions above it, like your proposed wrapper above. We purposefully did not try to write a higher level driver, since the space of designs is too large. If you think of the C++11 interface as more of a toolkit for building your own domain or application specific abstractions, I think some of the decisions will seem more reasonable. In many cases, had we made an opinionated choice, we would have helped some users at the expense of others.

Thanks,

Andrew

 

To view this discussion on the web visit https://groups.google.com/d/msgid/mongodb-user/103cd857-669b-478c-9bd4-269dbe72b34a%40googlegroups.com.

[li ning]

Understood

在 2016年6月21日星期二 UTC+8下午9:09:21，acm写道：