Small code review

[Christopher Benson-Manica]

Assuming the input to this program is

1
2
3
0
2
3
4
0

, is its behavior guaranteed to be well-defined? Is there anything
else about it that warrants comments and/or ridicule? The idea is to
accept two lists of numbers (separated by 0's) and print the numbers
that are in exactly one of the two lists; it seems to work.

#include <iostream>
#include <set>

int main()
{
unsigned int foo;
std::set< unsigned int > actual;
std::set< unsigned int > etb;
std::cin >> foo;
while( foo ) {
actual.insert( foo );
std::cin >> foo;
}
std::cin >> foo;
while( foo ) {
etb.insert( foo );
std::cin >> foo;
}
for( std::set<unsigned int>::iterator i=actual.begin() ; i !=
actual.end() ; ) {
std::set<unsigned int>::iterator j=etb.find( *i );
if( j != etb.end() ) {
std::set<unsigned int>::iterator del=i;
++i;
actual.erase( del );
etb.erase( j );
}
else {
++i;
}
}
for( std::set<unsigned int>::iterator i=actual.begin() ; i !=
actual.end() ; i++ ) {
std::cout << "actual: " << *i << std::endl;
}
for( std::set<unsigned int>::iterator i=etb.begin() ; i !=
etb.end() ; i++ ) {
std::cout << "etb: " << *i << std::endl;
}
return 0;
}

 

[Victor Bazarov]

Assuming the input to this program is

1
2
3
0
2
3
4
0

, is its behavior guaranteed to be well-defined?

Methinks, yes.

Is there anything
else about it that warrants comments and/or ridicule?

See inline with code. Nothing to ridicule. Just two small things
I'd do differently.

The idea is to
accept two lists of numbers (separated by 0's) and print the numbers
that are in exactly one of the two lists; it seems to work.

#include <iostream>
#include <set>

int main()
{
unsigned int foo;
std::set< unsigned int > actual;
std::set< unsigned int > etb;
std::cin >> foo;
while( foo ) {
actual.insert( foo );
std::cin >> foo;
}
std::cin >> foo;
while( foo ) {
etb.insert( foo );
std::cin >> foo;
}
for( std::set<unsigned int>::iterator i=actual.begin() ; i !=
actual.end() ; ) {
std::set<unsigned int>::iterator j=etb.find( *i );
if( j != etb.end() ) {
std::set<unsigned int>::iterator del=i;
++i;

You could easily do it in one statement:

actual.erase( del );
etb.erase( j );
}
else {
++i;
}

I personally don't like extraneous curly braces. Personally.

}
for( std::set<unsigned int>::iterator i=actual.begin() ; i !=
actual.end() ; i++ ) {
std::cout << "actual: " << *i << std::endl;
}
for( std::set<unsigned int>::iterator i=etb.begin() ; i !=
etb.end() ; i++ ) {
std::cout << "etb: " << *i << std::endl;
}
return 0;
}

V

 

[John Harrison]

Assuming the input to this program is

1
2
3
0
2
3
4
0

, is its behavior guaranteed to be well-defined? Is there anything
else about it that warrants comments and/or ridicule? The idea is to
accept two lists of numbers (separated by 0's) and print the numbers
that are in exactly one of the two lists; it seems to work.

#include <iostream>
#include <set>

int main()
{
unsigned int foo;
std::set< unsigned int > actual;
std::set< unsigned int > etb;
std::cin >> foo;
while( foo ) {
actual.insert( foo );
std::cin >> foo;
}
std::cin >> foo;
while( foo ) {
etb.insert( foo );
std::cin >> foo;
}
for( std::set<unsigned int>::iterator i=actual.begin() ; i !=
actual.end() ; ) {
std::set<unsigned int>::iterator j=etb.find( *i );
if( j != etb.end() ) {
std::set<unsigned int>::iterator del=i;
++i;
actual.erase( del );
etb.erase( j );
}
else {
++i;
}
}
for( std::set<unsigned int>::iterator i=actual.begin() ; i !=
actual.end() ; i++ ) {
std::cout << "actual: " << *i << std::endl;
}
for( std::set<unsigned int>::iterator i=etb.begin() ; i !=
etb.end() ; i++ ) {
std::cout << "etb: " << *i << std::endl;
}

This is one of the occasions when the STL really does work. The above three
loops could be written as a couple of lines

std:stream_iterator<unsigned int> output(std::cout, "\n");
std::set_symmetric_difference(actual.begin( ), actual.end( ), etb.begin( ),
etb.end( ), output);

You would lose the output "actual: " and "etb: " but if you were worried
about that then try this

std:stream_iterator<unsigned int> output(std::cout, "\n");
std::cout << "actual: ";
std::set_difference(actual.begin(), actual.end(), etb.begin(), etb.end(),
output);
std::cout << "etb: ";
std::set_difference(etb.begin(), etb.end(), actual.begin(), actual.end(),
output);

That's pretty close to what you had.

john