Hi ,

I am facing the following problem with new vc++ compiler 2005 , the
following code works well in 2003 compiler but fails with an error
saying sequence not ordered.

struct ltstr
{
     bool operator()(const string s1, string s2) const
     {
         return strcmp(s1.c_str(), s2.c_str()) < 0;
     }
};

int main()
{
     map<const char*,int,ltstr> mm1;
     map<const char*,int,ltstr> mm2;
     map<const char*,int,ltstr> umm;

     mm1["R01"]=1;
     mm1["RA1"]=2;
     mm1["RA2"]=3;
     mm1["RB1"]=4;
     mm1["RT1"]=5;

     mm2["R01"]=1;
     mm2["RA1"]=2;
     mm2["RA2"]=3;
     mm2["RA3"]=4;


set_union(mm1.begin(),mm1.end(),mm2.begin(),mm2.end(),inserter(umm,umm.begin()));

}

Is there something wrong with the way to write the code in VC++ 2005.

Thanks
Mani


-- 
      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

On 20 Sep., 20:46, Mani <mmanigan...@gmail.com> wrote:
> I am facing the following problem with new vc++ compiler 2005 , the
> following code works well in 2003 compiler but fails with an error
> saying sequence not ordered.

The compiler is right, there are several issues with your code.


> struct ltstr
> {
>      bool operator()(const string s1, string s2) const
>      {
>          return strcmp(s1.c_str(), s2.c_str()) < 0;
>      }
>
> };

It's rather confusing that you use a key type of const char* and
a comparator, which accepts std::string arguments - worse, these are
provided "by-value", instead of "by-(const)-reference". Note that this
is just a basic recommendation, which is not the reason of your
problem
- just a blatant design issue. Proposal for this: Replace ltstr with:

struct ltstr
{
     bool operator()(const char* s1, const char* s2) const
     {
          return strcmp(s1, s2) < 0;
     }
};

> int main()
> {
>      map<const char*,int,ltstr> mm1;
>      map<const char*,int,ltstr> mm2;
>      map<const char*,int,ltstr> umm;
>
>      mm1["R01"]=1;
>      mm1["RA1"]=2;
>      mm1["RA2"]=3;
>      mm1["RB1"]=4;
>      mm1["RT1"]=5;
>
>      mm2["R01"]=1;
>      mm2["RA1"]=2;
>      mm2["RA2"]=3;
>      mm2["RA3"]=4;
>
> set_union(mm1.begin(),mm1.end(),mm2.begin(),mm2.end(),inserter(umm,umm.begin()));
>
> }
>

Here are two issues:

You seem to misunderstand the reason for std::set_union -
it is not provided for std::map & Co (because they would
not need that function), they are provided for any general
sorted sequence (e.g. sorted vectors). The easy replacement
for std::map would be the following two lines instead:

     umm.insert(mm1.begin(), mm1.end());
     umm.insert(mm2.begin(), mm2.end());

But for the moment we let your code as it is and ignore its
misusage of set_union, because this is also *not* the reason
for your described problem.

The basic requirement for set_union is, that both sequences
are sorted according to the same order criterion. Since you
used the overload of set_union w/o predicate, it will apply
operator< to the value types of the sequences. These value
types are instances of std::pair<const char* const, int>
which has an operator <, but this operator uses operator <
for const char* and int, which is *not* the order you want.
For proper ordering you can use the same predicate that
std::map uses for it's own internal ordering:

    set_union(mm1.begin(),mm1.end(),mm2.begin(),mm2.end(),
      inserter(umm,umm.begin()), mm1.value_comp());

You can either use mm1.value_comp() or mm2.value_comp() or
even umm.value_comp() in this special case, because these
predicates have the same state. This call should result have
the equivalent result of the above mentioned (and strongly
recommended!) two lines

     umm.insert(mm1.begin(), mm1.end());
     umm.insert(mm2.begin(), mm2.end());

Greetings from Bremen,

Daniel Kr�gler



-- 
      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]

On Sep 21, 3:23 am, Daniel Kr|gler <daniel.krueg...@googlemail.com>
wrote:
> On 20 Sep., 20:46, Mani <mmanigan...@gmail.com> wrote:
>
> > I am facing the following problem with new vc++ compiler 2005 , the
> > following code works well in 2003 compiler but fails with an error
> > saying sequence not ordered.
>
> The compiler is right, there are several issues with your code.
>
> > struct ltstr
> > {
> >      bool operator()(const string s1, string s2) const
> >      {
> >          return strcmp(s1.c_str(), s2.c_str()) < 0;
> >      }
>
> > };
>
> It's rather confusing that you use a key type of const char* and
> a comparator, which accepts std::string arguments - worse, these are
> provided "by-value", instead of "by-(const)-reference". Note that this
> is just a basic recommendation, which is not the reason of your
> problem
> - just a blatant design issue. Proposal for this: Replace ltstr with:
>
> struct ltstr
> {
>      bool operator()(const char* s1, const char* s2) const
>      {
>           return strcmp(s1, s2) < 0;
>      }
>
>
>
> };
> > int main()
> > {
> >      map<const char*,int,ltstr> mm1;
> >      map<const char*,int,ltstr> mm2;
> >      map<const char*,int,ltstr> umm;
>
> >      mm1["R01"]=1;
> >      mm1["RA1"]=2;
> >      mm1["RA2"]=3;
> >      mm1["RB1"]=4;
> >      mm1["RT1"]=5;
>
> >      mm2["R01"]=1;
> >      mm2["RA1"]=2;
> >      mm2["RA2"]=3;
> >      mm2["RA3"]=4;
>
> >
set_union(mm1.begin(),mm1.end(),mm2.begin(),mm2.end(),inserter(umm,umm.begin
()));
>
> > }
>
> Here are two issues:
>
> You seem to misunderstand the reason for std::set_union -
> it is not provided for std::map & Co (because they would
> not need that function), they are provided for any general
> sorted sequence (e.g. sorted vectors). The easy replacement
> for std::map would be the following two lines instead:
>
>      umm.insert(mm1.begin(), mm1.end());
>      umm.insert(mm2.begin(), mm2.end());
>
> But for the moment we let your code as it is and ignore its
> misusage of set_union, because this is also *not* the reason
> for your described problem.
>
> The basic requirement for set_union is, that both sequences
> are sorted according to the same order criterion. Since you
> used the overload of set_union w/o predicate, it will apply
> operator< to the value types of the sequences. These value
> types are instances of std::pair<const char* const, int>
> which has an operator <, but this operator uses operator <
> for const char* and int, which is *not* the order you want.
> For proper ordering you can use the same predicate that
> std::map uses for it's own internal ordering:
>
>     set_union(mm1.begin(),mm1.end(),mm2.begin(),mm2.end(),
>       inserter(umm,umm.begin()), mm1.value_comp());
>
> You can either use mm1.value_comp() or mm2.value_comp() or
> even umm.value_comp() in this special case, because these
> predicates have the same state. This call should result have
> the equivalent result of the above mentioned (and strongly
> recommended!) two lines
>
>      umm.insert(mm1.begin(), mm1.end());
>      umm.insert(mm2.begin(), mm2.end());
>
> Greetings from Bremen,
>
> Daniel Kr|gler
>

Hi ,

Thanks for the help..
I have figured out the problem.. here is the solution.

By passing our own comparator the issue is fixed..
here is the new correct code

struct ltstr
{
    bool operator()(const char* s1, const char* s2) const
    {
         return strcmp(s1, s2) < 0;
    }
};

struct ltstr_pair_1st
{
    typedef pair<const char*,int> mypair;
    bool operator()(const mypair& s1, const mypair& s2) const
    {
        return strcmp(s1.first, s2.first) < 0;
    }
};

int main()
{
    map_type mm1;
    map_type mm2;
    map_type umm;

    mm1["R01"]=1;
    mm1["RA1"]=2;
    mm1["RA2"]=3;
    mm1["RB1"]=4;
    mm1["RT1"]=5;

    mm2["R01"]=1;
    mm2["RA1"]=2;
    mm2["RA2"]=3;
    mm2["RA3"]=4;

 
set_union(mm1.begin(),mm1.end(),mm2.begin(),mm2.end(),inserter(umm,umm.begin
()),
ltstr_pair_1st());

   return 0;
}

Mani



-- 
      [ See http://www.gotw.ca/resources/clcm.htm for info about ]
      [ comp.lang.c++.moderated.    First time posters: Do this! ]