Thorsten, 1. Since we can iterate over elements in container using pair of begin end or pair of ptr_begin ptr_end it's not enough to provide member-functions which take only iterators as arguments, we also need overloads which take ptr_iterators. Motivation example: ptr_vector<int> v; //... ptr_vector<int>::iterator i = std::find(v.begin(), v.end(), 5); if (i != v.end()) v.erase(i); //works just fine, but int k = 5; int * pk = &k; ptr_vector<int>::ptr_iterator i = std::find(v.ptr_begin(), v.ptr_end(), pk); if (i != v.ptr_end()) //here we can just write simple erase, we need to write something like that: v.erase(v.begin() + std::distance(v.ptr_begin(), i)); //and it's really no good. I'd like to list all the functions that need to have a twin, but I'd better say shorter - at least every public function that have one or more parameters with type iterator. Differently, ptr_iterators seem to be sick, petty and harmed. We just can't use them anywhere. Another, more simple approach is to provide function that returns iterator of the corresponding ptr_iterator. The funny thing is that we already can get ptr_iterator from iterator, but it's not so needed as the backward transformation. But I do think this is not a good solution, because with this approach we can achive the required functionality, but the syntax will be very very bad. One more approach is to simply get rid of ptr_iterators, but provide user with adapters he will ever need. I like it, but sometimes (or more oftener) I (and you) still simply need ptr_iterators. IMO the best solution is the hardest solution - approach number 1. 2. The next drill is on templated functions and constructors. If we will decide to keep ptr_iterators, consider the following code: ptr_vector<int> v; ptr_vector<int> v2(v.ptr_begin(), v.ptr_end()); IMO user (that was I) expects this compiling. But it's not. Why can't I construct ptr_vector from ptr_iterators? It's good iterators, just any else, but I can, because they need special care to handle them. IMO it's nonsense not to enable such support. The same applies to templated insert, assign, and other functions. So, all of them need an overload for templated ptr_iterators. I think the library should either supply the user of ptr_iterators all he will every need or get rid of them completely. No some more remarks, but they are not so important as the above. 4. IMO header <boost/ptr_container/clone_allocator.hpp> deserves to be higher in folder hierarchy since its' contents can be widely used by other libraries/people. 5. IMO header <boost/ptr_container/exception.hpp> contents doesn't follow good naming scheme. What do other people think when they see boost::bad_index & ? Some class, maybe exception, since it has prefix bad, but how knows that this class comes with ptr_container library? No-one. BTW why don't you use std::out_of_range? At least please change its' name. The same for boost::bad_pointer in the same header file. 6. What a funny name!! boost::ptr_container_ptr_container_detail in <boost/ptr_container/nullable.hpp> Maybe this header also deserves more fame(I mean nullable<T>)? 7. Why don't you throw std::out_of_range exception like std containers when ptr_sequence_adapter::at is out of bounds? Instead, you throw unknown boost::bad_index. 8. It's seems you didn't put contents of detail headers in detail namespace so we have boost::map_config class in <boost/ptr_container/ptr_map_adapter.hpp>. Why don't you put all adapters and their config classes to ptr_container::detail namespace? I don't think anyone will use them. 9. <boost/ptr_container/ptr_predicate.hpp> contents derserve to be documented. And it's better to make ptr versions to all <functional> functions and add these classes: template<typename T, typename U, typename R> class boost::ptr_binary_function : std::binary_funtion<T *, U *, R> {}; template<typename T, typename R> class boost::ptr_unary_function : std::unary_funtion<T *, R> {}; It will simplify your code and IMO it will be useful for users who wants to make their own ptr functions/predicates. 10. indirected1, indirected2 are not good names, don't you think? 11. std::set doesn't have set::at, why does boost::ptr_set have one(even two)? Sorry to dig so deep. P.S. BTW, have you fixed that const_ptr_iterator bug yet? -- Pavel Chikulaev
Hi Pavel, Finally someone who comments :-) "Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message news:d33fum$rdh$1@sea.gmane.org... | Thorsten, | | 1. Since we can iterate over elements in container using pair of begin end or | pair of ptr_begin ptr_end it's not enough to provide member-functions which | take only iterators as arguments, we also need overloads which take | ptr_iterators. | | Motivation example: | | ptr_vector<int> v; | //... | ptr_vector<int>::iterator i = std::find(v.begin(), v.end(), 5); | if (i != v.end()) | v.erase(i); //works just fine, but | | int k = 5; | int * pk = &k; | ptr_vector<int>::ptr_iterator i = std::find(v.ptr_begin(), v.ptr_end(), pk); | if (i != v.ptr_end()) | //here we can just write simple erase, we need to write something like that: | v.erase(v.begin() + std::distance(v.ptr_begin(), i)); | //and it's really no good. well, you can't normally make a search like that unless you use the view-clone allocator. | I'd like to list all the functions that need to have a twin, but I'd better say | shorter - at least every public function that have one or more parameters | with type iterator. Differently, ptr_iterators seem to be sick, petty and | harmed. We just can't use them anywhere. the intention was that they could be used with permutating, mutating standard algorithms, eg. sort(). you need access to the pointer to be able to do that. | Another, more simple approach is to provide function that returns iterator | of the corresponding ptr_iterator. | The funny thing is that we already can | get ptr_iterator from iterator, but it's not so needed as the backward | transformation. But I do think this is not a good solution, because with this | approach we can achive the required functionality, but the syntax will be | very very bad. | | One more approach is to simply get rid of ptr_iterators, but provide user | with adapters he will ever need. I like it, but sometimes (or more oftener) | I (and you) still simply need ptr_iterators. | | IMO the best solution is the hardest solution - approach number 1. well, the function that currently turn a ptr_iterator into an iterator is called the constructor of an iterator. As you say, ptr_iterators seems to be sick...in fact, the only legal solution is to simply let them be iterators over void*'s. yes, the current implementation might work on 99% of all compilers, but it is not guaranteed to do so. So that has to happen. Given that, do we still want them? The alternatives are 1. yes, provide them and let users do the cast inside their functor 2. no, don't provide them 2. no, don't provide them, but implement safe versions of the most common algorithms, eg. sort unique merge erase erase_if | 2. The next drill is on templated functions and constructors. | If we will decide to keep ptr_iterators, consider the following code: well, let's dezide that first, then. | 4. IMO header <boost/ptr_container/clone_allocator.hpp> | deserves to be higher in folder hierarchy since its' contents | can be widely used by other libraries/people. I'll consider this. | 5. IMO header <boost/ptr_container/exception.hpp> contents doesn't follow good | naming scheme. | What do other people think when they see boost::bad_index & ? | Some class, maybe exception, since it has prefix bad, but how knows | that this class comes with ptr_container library? No-one. BTW | why don't you use std::out_of_range? because it takes an std::string argument in its constructor. | At least please change its' name. | The same for boost::bad_pointer in the same header file. isn't the important part that we inherit from std::exception? what names do you suggest? | 6. What a funny name!! | boost::ptr_container_ptr_container_detail in | <boost/ptr_container/nullable.hpp> ah, a search and replace bug. | Maybe this header also deserves more fame(I mean nullable<T>)? perhaps. | 7. Why don't you throw std::out_of_range exception like std containers when | ptr_sequence_adapter::at is out of bounds? Instead, you throw unknown | boost::bad_index. because of the string argument | 8. It's seems you didn't put contents of detail headers in detail namespace so | we have boost::map_config class in | <boost/ptr_container/ptr_map_adapter.hpp>. | Why don't you put all adapters and their config classes to | ptr_container::detail | namespace? I don't think anyone will use them. it was requested during the review. I'm not too keen on the idea, but it doesn't hurt presently. I think you're right that map_config and ptr_map_adapter_base ect should go into ptr_container_detail namespace. | 9. <boost/ptr_container/ptr_predicate.hpp> contents derserve to be documented. actually, it should be removed. | And it's better to make ptr versions to all <functional> functions and add | these | classes: | template<typename T, typename U, typename R> | class boost::ptr_binary_function : std::binary_funtion<T *, U *, R> {}; | template<typename T, typename R> | class boost::ptr_unary_function : std::unary_funtion<T *, R> {}; | It will simplify your code and IMO it will be useful for users who wants | to make their own ptr functions/predicates. | | 10. indirected1, indirected2 are not good names, don't you think? please take a look at indirect_fun; it is meant to replace the ptr_predicate header. | 11. std::set doesn't have set::at, why does boost::ptr_set have one(even two)? I haven't documented it yet because I was doubtful about it too. I all for removing it. | Sorry to dig so deep. no, thank you for digging so deep. | P.S. BTW, have you fixed that const_ptr_iterator bug yet? about conversion from ptr_iterator? If so yes, -Thorsten
"Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message news:d33saa$bn2$1@sea.gmane.org...
well, you can't normally make a search like that unless you use the view-clone allocator.
What you mean? if we have erase erase(ptr_iterator), shouldn't erase(i) work properly despite current allocator?
| I'd like to list all the functions that need to have a twin, but I'd better say | shorter - at least every public function that have one or more parameters | with type iterator. Differently, ptr_iterators seem to be sick, petty and | harmed. We just can't use them anywhere.
the intention was that they could be used with permutating, mutating standard algorithms, eg. sort(). you need access to the pointer to be able to do that.
I see. The more I think about it, the more I think that we should remove ptr_iterators. Because, on the one hand, they are iterators of elements in container, but we can't make a copy of container using these iterators, and we just can't use them almost anywhere, on the other hand, if we provide them with functions we can put them in, changes in std algorithms are also needed to unify usage, but since we can't do it, there is no place for ptr_iterators at all. We can always use lambda: int * p; std::find_if(v.begin(), v.end(), &_1 == p);
| Another, more simple approach is to provide function that returns iterator | of the corresponding ptr_iterator. | The funny thing is that we already can | get ptr_iterator from iterator, but it's not so needed as the backward | transformation. But I do think this is not a good solution, because with this | approach we can achive the required functionality, but the syntax will be | very very bad. | | One more approach is to simply get rid of ptr_iterators, but provide user | with adapters he will ever need. I like it, but sometimes (or more oftener) | I (and you) still simply need ptr_iterators. | | IMO the best solution is the hardest solution - approach number 1.
well, the function that currently turn a ptr_iterator into an iterator is called the constructor of an iterator.
As you say, ptr_iterators seems to be sick...in fact, the only legal solution is to simply let them be iterators over void*'s. yes, the current implementation might work on 99% of all compilers, but it is not guaranteed to do so.
So that has to happen. Given that, do we still want them? The alternatives are
1. yes, provide them and let users do the cast inside their functor
2. no, don't provide them
I vote for this.
2. no, don't provide them, but implement safe versions of the most common algorithms, eg.
sort unique merge erase erase_if
| 2. The next drill is on templated functions and constructors. | If we will decide to keep ptr_iterators, consider the following code:
well, let's dezide that first, then.
It's not needed anymore.
| 4. IMO header <boost/ptr_container/clone_allocator.hpp> | deserves to be higher in folder hierarchy since its' contents | can be widely used by other libraries/people.
I'll consider this.
ok.
| 5. IMO header <boost/ptr_container/exception.hpp> contents doesn't follow good | naming scheme. | What do other people think when they see boost::bad_index & ? | Some class, maybe exception, since it has prefix bad, but how knows | that this class comes with ptr_container library? No-one. BTW | why don't you use std::out_of_range?
because it takes an std::string argument in its constructor.
| At least please change its' name. | The same for boost::bad_pointer in the same header file.
isn't the important part that we inherit from std::exception?
what names do you suggest?
First, I suggest putting everything in ptr_container namespace. Second, classes that are not likely going to be used by other people put in ptr_container::namespace. And in each header <boost/ptr_container/ptr_X.hpp> promote X to boost namespace. (Where X are one of these: vector, list, deque, map, set, etc) Also I'd like to tell you what I think about namespaces. Most people (and it seems you too) think that if they put their classes in namespace, no problems with namespaces is ever going to be happed. But it's not. In our example the namespace is called 'boost'. So if every boost developer will put their classes just in namespace 'boost' without structurizing it would be same problems as C has with names. In your case, everything should be in ptr_container namespace, and only the top of the iceberg should be above water, in namespace boost.
| 6. What a funny name!! | boost::ptr_container_ptr_container_detail in | <boost/ptr_container/nullable.hpp>
ah, a search and replace bug.
| Maybe this header also deserves more fame(I mean nullable<T>)?
perhaps.
Now I think it doesn't deserve any. It serves only one serve: to tell the pointer container that null pointers are not error. And we have optional<T> (at least in runtime)
| 7. Why don't you throw std::out_of_range exception like std containers when | ptr_sequence_adapter::at is out of bounds? Instead, you throw unknown | boost::bad_index.
because of the string argument
About bad_index: What string argument? You have same char *. Even if you make ctor with std::string, and if you user catchs only std::exception, he is going to get information using const char * what(), so std::string is not needed here. So, I don't see ANY reason not to use std::out_of_range! About bad_pointer: It's better to derive from std::runtime_error or even throw std::runtime_error("ptr_container: null pointer not allowed"); You don't ever throw bad_ptr_container_operation, do you? (BTW, maybe it should be better called ptr_container::bad_operation? because your ptr_containers are not so bad :)) I think you shouldn't introduce and use any own classes derived from std::exception, because your containers are only extension to std containers that simplifies using of pointers in containers. So everybody expects it to work as same as std::containers when possible. It applies not only to exceptions but all member functions and so on. Example from std: Everybody expects(expected) vector<bool> to be same as any other vector<T> specialization, but it is not. Now, IFAIK almost nobody use it, and I really don't want your library to repeat vector<bool> failure.
| 8. It's seems you didn't put contents of detail headers in detail namespace so | we have boost::map_config class in | <boost/ptr_container/ptr_map_adapter.hpp>. | Why don't you put all adapters and their config classes to | ptr_container::detail | namespace? I don't think anyone will use them.
it was requested during the review. I'm not too keen on the idea, but it doesn't hurt presently. I think you're right that map_config and ptr_map_adapter_base ect should go into ptr_container_detail namespace.
If I were you, I even go further - I would put everything in detail namespace except ptr_XXXX classes and allocators.
| 9. <boost/ptr_container/ptr_predicate.hpp> contents derserve to be documented.
actually, it should be removed.
| And it's better to make ptr versions to all <functional> functions and add | these | classes: | template<typename T, typename U, typename R> | class boost::ptr_binary_function : std::binary_funtion<T *, U *, R> {}; | template<typename T, typename R> | class boost::ptr_unary_function : std::unary_funtion<T *, R> {}; | It will simplify your code and IMO it will be useful for users who wants | to make their own ptr functions/predicates. | | 10. indirected1, indirected2 are not good names, don't you think?
Since their operators () return bool we can call them indirected_unary_predicate and indirected_binary_predicate. But since we have indirect_fun we better remove them all.
please take a look at indirect_fun; it is meant to replace the ptr_predicate header.
I've already taken a look, but I simply had no comments about it. But now I do. BTW indirect_fun is needed only if user uses ptr_iterator, but with their current capabilities, user won't use neither ptr_begin-ptr_end nor indirect_fun. Ex: std::sort(v.ptr_begin(), v.ptr_end(), indirect_fun<greater<int> >()); std::sort(v.begin(), v.end(), greater<int>()); IMO it's better to use lambda: std::sort(v.ptr_begin(), v.ptr_end(), *_1 > *_2); Don't you think this syntax is much more readable/flexible/extensible and so on? I just don't see advantages of indirect_fun, may be you do?
| 11. std::set doesn't have set::at, why does boost::ptr_set have one(even two)? I haven't documented it yet because I was doubtful about it too. I all for removing it.
Looking forward to it. BTW what was the reason you added them?
| P.S. BTW, have you fixed that const_ptr_iterator bug yet? about conversion from ptr_iterator? If so yes,
Thanks, can you upload a fixed version to sandbox? -- Pavel Chikulaev
"Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message news:d34249$jo$1@sea.gmane.org... | "Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message | news:d33saa$bn2$1@sea.gmane.org... | > well, you can't normally make a search like that unless you use the view-clone | > allocator. | | What you mean? | if we have erase erase(ptr_iterator), shouldn't erase(i) work properly | despite | current allocator? yes, but I was commenting the search for the address of an element on the stack. | > | > | I'd like to list all the functions that need to have a twin, but I'd better | > say | > | shorter - at least every public function that have one or more parameters | > | with type iterator. Differently, ptr_iterators seem to be sick, petty and | > | harmed. We just can't use them anywhere. | > | > the intention was that they could be used with permutating, mutating standard | > algorithms, | > eg. sort(). you need access to the pointer to be able to do that. | | I see. The more I think about it, the more I think that we should remove | ptr_iterators. I have a similar feeling. | > | IMO the best solution is the hardest solution - approach number 1. | > | > well, the function that currently turn a ptr_iterator into an iterator is | > called the constructor of an iterator. | > | > As you say, ptr_iterators seems to be sick...in fact, the only legal | > solution is to simply let them be iterators over void*'s. yes, the current | > implementation might work on 99% of all compilers, but it is not | > guaranteed to do so. | > | > So that has to happen. Given that, do we still want them? The alternatives | > are | > | > 1. yes, provide them and let users do the cast inside their functor | > | > 2. no, don't provide them | | I vote for this. | > | > 2. no, don't provide them, but implement safe versions of the most common | > algorithms, eg. | > | > sort | > unique | > merge | > erase | > erase_if | > you don't want the algorithms? | > | 5. IMO header <boost/ptr_container/exception.hpp> contents doesn't follow | > good | > | naming scheme. | > | What do other people think when they see boost::bad_index & ? | > | Some class, maybe exception, since it has prefix bad, but how knows | > | that this class comes with ptr_container library? No-one. BTW | > | why don't you use std::out_of_range? | > | > because it takes an std::string argument in its constructor. | > | > | At least please change its' name. | > | The same for boost::bad_pointer in the same header file. | > | > isn't the important part that we inherit from std::exception? | > | > what names do you suggest? | First, I suggest putting everything in ptr_container namespace. I meant the name of the exceptions. | Second, classes that are not likely going to be used by other | people put in ptr_container::namespace. | And in each header <boost/ptr_container/ptr_X.hpp> promote | X to boost namespace. (Where X are one of these: vector, list, | deque, map, set, etc) | | Also I'd like to tell you what I think about namespaces. | Most people (and it seems you too) think that if they put their | classes in namespace, no problems with namespaces is ever | going to be happed. But it's not. In our example the namespace | is called 'boost'. So if every boost developer will put their classes | just in namespace 'boost' without structurizing it would be same | problems as C has with names. In your case, everything should | be in ptr_container namespace, and only the top of the iceberg | should be above water, in namespace boost. yeah, well, there is not going to many other ptr_vector<T> in boost. | > | Maybe this header also deserves more fame(I mean nullable<T>)? | > | > perhaps. | | Now I think it doesn't deserve any. It serves only one serve: to tell the | pointer | container that null pointers are not error. | And we have optional<T> (at least in runtime) nullable<T> was the best compromize I could find for the behavior. Adding another policy seemed overkill. optional<T> has some parallels, but it would not have the same meaning if we wrote ptr_vector< optional<T> >. ptr_vector<nullable<T>> is something similar to vector<optional<T>>. | > | > | 7. Why don't you throw std::out_of_range exception like std containers when | > | ptr_sequence_adapter::at is out of bounds? Instead, you throw unknown | > | boost::bad_index. | > | > because of the string argument | About bad_index: | What string argument? You have same char *. Even if you make ctor with | std::string, and if you user catchs only std::exception, he is going to | get | information using const char * what(), so std::string is not needed | here. | So, I don't see ANY reason not to use std::out_of_range! the copy-constructor of std::string might throw; hence when constructing the bad_index exception-object, we might not get that far and the *wrong* error will be reported. | About bad_pointer: | It's better to derive from std::runtime_error or even throw | std::runtime_error("ptr_container: null pointer not allowed"); | You don't ever throw bad_ptr_container_operation, do you? | (BTW, maybe it should be better called ptr_container::bad_operation? | because your ptr_containers are not so bad :)) | | I think you shouldn't introduce and use any own classes derived from | std::exception, because your containers are only extension to std containers | that simplifies using of pointers in containers. somebody might want to catch a more specific exception. | So everybody expects it | to work as same as std::containers when possible. | It applies not only to exceptions but all member functions and so on. | Example from std: Everybody expects(expected) vector<bool> to be same | as any other vector<T> specialization, but it is not. Now, IFAIK almost | nobody use it, and I really don't want your library to repeat vector<bool> | failure. I don't think the analogy is quite the same. | > | 8. It's seems you didn't put contents of detail headers in detail namespace | > so | > | we have boost::map_config class in | > | <boost/ptr_container/ptr_map_adapter.hpp>. | > | Why don't you put all adapters and their config classes to | > | ptr_container::detail | > | namespace? I don't think anyone will use them. | > | > it was requested during the review. I'm not too keen on the idea, but | > it doesn't hurt presently. | > I think you're right that map_config and ptr_map_adapter_base ect should | > go into ptr_container_detail namespace. | | If I were you, I even go further - I would put everything in detail | namespace | except ptr_XXXX classes and allocators. yeah, maybe. I'll think about it. | > | > | 9. <boost/ptr_container/ptr_predicate.hpp> contents derserve to be | > documented. | > | > actually, it should be removed. | > | > | And it's better to make ptr versions to all <functional> functions and | > add | > | these | > | classes: | > | template<typename T, typename U, typename R> | > | class boost::ptr_binary_function : std::binary_funtion<T *, U *, R> | > {}; | > | template<typename T, typename R> | > | class boost::ptr_unary_function : std::unary_funtion<T *, R> {}; | > | It will simplify your code and IMO it will be useful for users who wants | > | to make their own ptr functions/predicates. | > | | > | 10. indirected1, indirected2 are not good names, don't you think? | | Since their operators () return bool we can call them | indirected_unary_predicate and indirected_binary_predicate. | But since we have indirect_fun we better remove them all. | | > please take a look at indirect_fun; it is meant to replace the ptr_predicate | > header. | | I've already taken a look, but I simply had no comments about it. | But now I do. | | BTW indirect_fun is needed only if user uses ptr_iterator, but with | their current capabilities, user won't use neither ptr_begin-ptr_end nor | indirect_fun. that is true. When/If I remove ptr_iterator, this header could be kept an implementation detail. | Ex: | std::sort(v.ptr_begin(), v.ptr_end(), indirect_fun<greater<int> >()); | std::sort(v.begin(), v.end(), greater<int>()); | IMO it's better to use lambda: | std::sort(v.ptr_begin(), v.ptr_end(), *_1 > *_2); | Don't you think this syntax is much more readable/flexible/extensible | and so on? I just don't see advantages of indirect_fun, may be you do? lambda works ok when you have a simple function like <. Some people like the other approach I guess. | > | 11. std::set doesn't have set::at, why does boost::ptr_set have one(even | > two)? | > I haven't documented it yet because I was doubtful about it too. I all for | > removing it. | | Looking forward to it. BTW what was the reason you added them? maybe because they are in map, but thinking about them, I don't think they make much sense in ptr_set. | > | P.S. BTW, have you fixed that const_ptr_iterator bug yet? | > about conversion from ptr_iterator? If so yes, | | Thanks, can you upload a fixed version to sandbox? The main-cvs version is somewhat newer; you can access that one, right? br -Thorsten
"Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message news:d343vc$7ve$1@sea.gmane.org...
you don't want the algorithms? Bad worded. Since I think that should ptr_iterator should be removed, no specialized versions of algorithms are needed.
I meant the name of the exceptions. Ok, not so shorty then, more descriptable. Or shorty, but in namespace ptr_container.
yeah, well, there is not going to many other ptr_vector<T> in boost. What about map_config? Additional security (almost for free) won't be bad. (I mean putting in own namespace)
nullable<T> was the best compromize I could find for the behavior. Adding another policy seemed overkill. optional<T> has some parallels, but it would not have the same meaning if we wrote ptr_vector< optional<T> >. ptr_vector<nullable<T>> is something similar to vector<optional<T>>.
I meant in other's code nullable<T> is not needed. But I understand that optional<T> and nullable<T> are completly different. BTW, in other languages (C#) have embedded nullable<T> which works same as boost::optional<T>. :)
the copy-constructor of std::string might throw; hence when constructing the bad_index exception-object, we might not get that far and the *wrong* error will be reported.
Still can't get your point. std::exception uses char *, your_exception_classes use char *, what std::strings are you talking about?
| About bad_pointer: | It's better to derive from std::runtime_error or even throw | std::runtime_error("ptr_container: null pointer not allowed"); | You don't ever throw bad_ptr_container_operation, do you? | (BTW, maybe it should be better called ptr_container::bad_operation? | because your ptr_containers are not so bad :)) | | I think you shouldn't introduce and use any own classes derived from | std::exception, because your containers are only extension to std containers | that simplifies using of pointers in containers.
somebody might want to catch a more specific exception.
Then derive from runtime_error, but not exception.
| So everybody expects it | to work as same as std::containers when possible. | It applies not only to exceptions but all member functions and so on. | Example from std: Everybody expects(expected) vector<bool> to be same | as any other vector<T> specialization, but it is not. Now, IFAIK almost | nobody use it, and I really don't want your library to repeat vector<bool> | failure. I don't think the analogy is quite the same.
That was about ptr_iterators, map::at and so on.
| std::sort(v.ptr_begin(), v.ptr_end(), indirect_fun<greater<int> >()); | std::sort(v.begin(), v.end(), greater<int>()); | IMO it's better to use lambda: | std::sort(v.ptr_begin(), v.ptr_end(), *_1 > *_2); | Don't you think this syntax is much more readable/flexible/extensible | and so on? I just don't see advantages of indirect_fun, may be you do?
lambda works ok when you have a simple function like <. Some people like the other approach I guess.
Then they can use std::sort(v.ptr_begin(), v.ptr_end(), std::greater<int>()(*_1, *_2));
The main-cvs version is somewhat newer; you can access that one, right? Didn't know that library is already there. Thanks.
-- Pavel Chikulaev
"Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message news:d345s3$egm$1@sea.gmane.org... | "Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message | news:d343vc$7ve$1@sea.gmane.org... | > you don't want the algorithms? | Bad worded. Since I think that should ptr_iterator should be removed, no | specialized versions of algorithms are needed. | | > I meant the name of the exceptions. | Ok, not so shorty then, more descriptable. Or shorty, but in namespace | ptr_container. ok. | > yeah, well, there is not going to many other ptr_vector<T> in boost. | What about map_config? Additional security (almost for free) won't | be bad. (I mean putting in own namespace) it sounds fine. | > the copy-constructor of std::string might throw; hence when constructing | > the bad_index exception-object, we might not get that far and the *wrong* | > error will be reported. | | Still can't get your point. std::exception uses char *, your_exception_classes | use char *, what std::strings are you talking about? std::out_of_range uses std::string. | Then derive from runtime_error, but not exception. same problem. | > I don't think the analogy is quite the same. | | That was about ptr_iterators, map::at and so on. std::map<K,T> will get at() in the next standard. -Thorsten
"Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message news:d34827$vod$1@sea.gmane.org...
std::out_of_range uses std::string. Didn't know about that. Just checked standard, that's true. Do you know why it is soo?
std::map<K,T> will get at() in the next standard. Where can I read about that?
So what's your decision about ptr_iterators - to be or not to be? -- Pavel Chikulaev
"Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message news:d3490f$ojo$1@sea.gmane.org... | "Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message | news:d34827$vod$1@sea.gmane.org... | > std::out_of_range uses std::string. | Didn't know about that. Just checked standard, that's true. | Do you know why it is soo? historical accident; the mechanism might not have been fully understood when the first standard was written. | > std::map<K,T> will get at() in the next standard. | Where can I read about that? http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2005/n1753.html#464 | So what's your decision about ptr_iterators - to be or not to be? let's scrap them and provide a few common algorithms. -Thorsten
"Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message news:d34bue$2ab$1@sea.gmane.org...
"Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message news:d3490f$ojo$1@sea.gmane.org... | "Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message | news:d34827$vod$1@sea.gmane.org... | > std::out_of_range uses std::string. | Didn't know about that. Just checked standard, that's true. | Do you know why it is soo?
historical accident; the mechanism might not have been fully understood when the first standard was written.
Are they going to be changed in the next standard? How else could they be implemented? When need you throw dynamically allocated string, something should store it and remove when needed - and std::string serves that great. But when you only need to throw statically allocated char *, there is some overhead, and something may happen then. Since you can't provide two different implementation of such classes (I mean copying string and referencing string), the existent way is the only way to do it. (Of cource we can use in_place, but that's a bad way to do it IMO). And what is more important is that we just can't have another way to do (I mean not-throw-guarantee) except malloc/free which is deprecated in C++. (And even if they were not deprecated, what() may return 0, I think it's not the behaviour users are expected to see. So, I think you should derive from std::runtime_error anyway. If i'm entirely wrong, can you provide a link where this behaviour of exception derived classes was discussed and the proposed solution for it?
| So what's your decision about ptr_iterators - to be or not to be? let's scrap them and provide a few common algorithms.
What algorithms? If you scap them, no additional algorithms are needed, or you didn't provide some? AFAIK all std algorithms works just fine with ptr_vector<T>::iterators, I just don't see any reason they shouldn't.
"Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message news:d35mcn$5pg$2@sea.gmane.org... | "Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message | news:d34bue$2ab$1@sea.gmane.org... | > "Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message | > news:d3490f$ojo$1@sea.gmane.org... | > | "Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message | > | news:d34827$vod$1@sea.gmane.org... | > | > std::out_of_range uses std::string. | > | Didn't know about that. Just checked standard, that's true. | > | Do you know why it is soo? | > | > historical accident; the mechanism might not have been fully understood when | > the first standard was written. | | Are they going to be changed in the next standard? | | How else could they be implemented? no idea. | When need you throw dynamically allocated string, something should store it | and remove when needed - and std::string serves that great. | But when you only need to throw statically allocated char *, there is some | overhead, and something may happen then. | Since you can't provide two different implementation of such classes | (I mean copying string and referencing string), the existent way is the | only way to do it. (Of cource we can use in_place, but that's a bad way to do | it IMO). And what is more important is that we just can't have another | way to do (I mean not-throw-guarantee) except malloc/free which is deprecated | in C++. (And even if they were not deprecated, what() may return 0, I think | it's not the behaviour users are expected to see. | | So, I think you should derive from std::runtime_error anyway. | | If i'm entirely wrong, can you provide a link where this behaviour of | exception derived classes was discussed and the proposed solution for it? | | > | So what's your decision about ptr_iterators - to be or not to be? | > let's scrap them and provide a few common algorithms. | | What algorithms? I'm thinking unique sort merge erase_if | If you scap them, no additional algorithms are needed, | or you didn't provide some? AFAIK all std algorithms works just fine with | ptr_vector<T>::iterators, I just don't see any reason they shouldn't. no. you can't, eg., write sort() when the type is copyable (which is very common). -Thorsten
| How else could they be implemented? no idea. So, maybe then derive from runtime_error, despite not safe ctors,
| If you scap them, no additional algorithms are needed, | or you didn't provide some? AFAIK all std algorithms works just fine with | ptr_vector<T>::iterators, I just don't see any reason they shouldn't.
no. you can't, eg., write sort() when the type is copyable (which is very common). Now I see, I think we can't think that some algorithms are more important
"Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message news:d3d7uk$mm2$1@sea.gmane.org... like from std containers? than others, so we should implement them all. Should they be implemented in namespace boost (for compilers that support Koenig lookup), or in namespace std as overloads? I can implement that algorithms, if you'd like to. -- Pavel Chikulaev
"Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message news:d3dqbf$gk6$1@sea.gmane.org... | "Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message | news:d3d7uk$mm2$1@sea.gmane.org... | > | How else could they be implemented? | > no idea. | So, maybe then derive from runtime_error, despite not safe ctors, | like from std containers? | | > | If you scap them, no additional algorithms are needed, | > | or you didn't provide some? AFAIK all std algorithms works just fine | > with | > | ptr_vector<T>::iterators, I just don't see any reason they shouldn't. | > | > no. you can't, eg., write sort() when the type is copyable (which is very | > common). | Now I see, it will work on some libraries where it is guaranteed that a user defined swap will be found. | I think we can't think that some algorithms are more important | than | others, so we should implement them all. well, I think there is a key number of few algorithms which cover 99.9 % or the use cases. In partucular, I think sort() merge() erase_if() unique() is good enough. | Should they be implemented in namespace boost (for compilers that support | Koenig lookup), | or in namespace std as overloads? they should be member functions to reflect their special semantics and to follow the example of std::list.. | I can implement that algorithms, if you'd like to. I've basically done it, but thanks anyway. -Thorsten
"Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message news:d3fqsr$k4o$3@sea.gmane.org...
"Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message it will work on some libraries where it is guaranteed that a user defined swap will be found.
Maybe introduce new traits class something like is_swappable? And then specialize it for Swap, swap, SwapMe user's class member functions? With this technique we can cover almost everything, and this will simplify implementing algorithms a lot. -- Pavel Chikulaev
"Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message news:d3g95g$rj8$1@sea.gmane.org... | "Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message | news:d3fqsr$k4o$3@sea.gmane.org... | > "Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message | > it will work on some libraries where it is guaranteed that a user defined swap | > will be found. | | Maybe introduce new traits class something like is_swappable? And then | specialize it for Swap, swap, SwapMe user's class member functions? | With this technique we can cover almost everything, and this will simplify | implementing algorithms a lot. This beyond what I'm trying to do. We can't just change all standard library implementations. -Thorsten
"Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message news:d345s3$egm$1@sea.gmane.org...
Then they can use std::sort(v.ptr_begin(), v.ptr_end(), std::greater<int>()(*_1, *_2)); Oops. I should be read as: std::sort(v.ptr_begin(), v.ptr_end(), bind(std::greater<int>(), *_1, *_2));
"Pavel Chikulaev" <pavel.chikulaev@gmail.com> wrote in message news:d34249$jo$1@sea.gmane.org...
what names do you suggest? First, I suggest putting everything in ptr_container namespace. Second, classes that are not likely going to be used by other people put in ptr_container::namespace. This should be read as ptr_container::detail namespace.
-- Pavel Chikulaev
participants (2)
-
Pavel Chikulaev -
Thorsten Ottosen