There's a couple of problems I can see here:
- you're not checking for the end of the file or something else going wrong with the stream.
- you're using the member version of getline, use the free function version instead
- using an extractor on a stream doesn't remove any white-space after it so getline will be reading an empty string
Use something like:
int read_integer_from_stream( std::istream &from )
{
int n; from >> n;
std::string rest_of_line;
std::getline( from, rest_of_line );
return n;
}
Oh, and don't use arrays, this isn't 1994. Try using std::vector and std::string - they're far safer. And presuming weapon's actually going to be used for something make it a class.
End of sermon :-)
PS: Actually it's not the end of the sermon... Looking at the previous solution and criticising it for not being C++ but a bastard breed of C and C++ I thought I should put my money where my mouth is and knock up a quick idea of how I'd approach the problem:
#include <sstream>
#include <vector>
#include <iterator>
#include <string>
int read_integer_from_stream( std::istream &from )
{
int n; from >> n;
std::string rest_of_line;
std::getline( from, rest_of_line );
return n;
}
class weapon
{
public:
weapon() : power_( 0 ){}
weapon( std::istream &initialise_from )
{
std::getline( initialise_from, name_ );
power_ = read_integer_from_stream( initialise_from );
}
weapon( weapon &©_me )
{
swap( std::move( copy_me ) );
}
weapon &operator=( const weapon ©_me )
{
weapon w( copy_me );
swap( std::move( w ) );
return *this;
}
weapon &operator=( weapon &©_me )
{
swap( std::move( copy_me ) );
return *this;
}
std::string name() const
{
return name_;
}
bool is_more_powerful_than( const weapon &compare_against ) const
{
return power_ > compare_against.power_;
}
void swap( weapon &&swap_with )
{
std::swap( swap_with.name_, name_ );
std::swap( swap_with.power_, power_ );
}
friend std::ostream &operator<<( std::ostream &, const weapon & );
friend std::ostream &operator>>( std::istream &, const weapon & );
private:
std::string name_;
int power_;
};
std::ostream &operator<<( std::ostream &str, const weapon &w )
{
return str << w.name_ << '\n' << w.power_ << '\n';
}
std::istream &operator>>( std::istream &str, weapon &to_modify )
{
weapon w( str );
to_modify.swap( std::move( w ) );
return str;
}
void weapon_test( std::ostream &output )
{
std::istringstream input( "Sword\n87\nAxe\n88\n" );
std::vector<weapon> weapons;
typedef std::istream_iterator<weapon> input_iter;
typedef std::ostream_iterator<weapon> output_iter;
std::copy( input_iter( input ), input_iter(), std::back_inserter( weapons ) );
std::copy( begin( weapons ), end( weapons ), output_iter( output, "" ) );
}
There are still some horrible things in here I wouldn't do it real life:
- All the functions are inline
- I'm not permissive enough about the stream format, each weapon has to end with a newline, even the last one
- I'm not using a PIMP
- The test harness is trival and, honestly, crap. Not every behaviour is tested (although they're fairly trivial)
- The class is in the same file as the test
- No non-member swap
Good points are (he says modestly...)
- No manual loops
- Class can cope with most things an intrinsic value can
- Has an exception safe assignment operaor
- Has a move constructor and assignment operator (really important these days)
Numerous Edits: To get rid of tabs and make it look slightly better, fix loads of errors caused by idiot me pasting an old version in