A acm problem. What's wrong in my code?
The text of problem:
Input: The input stream contains a set of integer numbers Ai (0 ≤ Ai ≤ 10开发者_运维知识库18). The numbers are separated by any number of spaces and line breaks. A size of the input stream does not exceed 256 KB.
Output: For each number Ai from the last one till the first one you should output its square root. Each square root should be printed in a separate line with at least four digits after decimal point.
Sample
input:1427 0876652098643267843 5276538
output:
2297.0716 936297014.1164 0.0000 37.7757
Time Limit: 2.0 second
Memory Limit: 16 MBC and C++ programs are compiled on the server by the 32-bit Microsoft Visual C++ 2010. Until August 3, 2010 Intel C++ Compiler 7.0 was used. You can download a free copy of Microsoft Visual Studio 2010 Express on this page. The compiler is invoked with the following parameters:
// C cl /TC /MT /EHsc /O2 /W3 /Za /D "_CRT_SECURE_NO_WARNINGS" /D "_CRT_SECURE_NO_DEPRECATE" /D "ONLINE_JUDGE" // C++ cl /TP /MT /EHsc /O2 /W3 /Za /D "_CRT_SECURE_NO_WARNINGS" /D "_CRT_SECURE_NO_DEPRECATE" /D "ONLINE_JUDGE"
This is my solution of this problem in gcc 4.3:
#include <iostream>
#include <list>
#include <string>
#include <sstream>
#include <math.h>
#include <algorithm>
#include <iomanip>
#include <stdio.h>
void sqrt_f(double n)
{
printf ( "%.4f\n", sqrt( static_cast<double>( n ) ) );
}
int main()
{
std::list<double> numbers;
std::string sInput;
getline( std::cin, sInput );
std::istringstream parse( sInput );
double tmp;
while ( parse >> tmp )
numbers.push_front( tmp );
std::for_each(numbers.begin(), numbers.end(), sqrt_f);
return 0;
}
But Judgement result is: wrong answer (test1)
What's the problem?
Warning: when I wrote this, the question was still on the code-review site, so it's written primarily as a code review. The part that applies primarily here on SO (fixing the input problem) is only barely mentioned in passing.
For the moment, I'm going to assume that you've fixed the problem @Jeff Mercado pointed out to get code something like this:
#include <iostream>
#include <list>
#include <string>
#include <sstream>
#include <math.h>
#include <algorithm>
#include <iomanip>
#include <stdio.h>
void sqrt_f(double n)
{
printf ( "%.4f\n", sqrt( static_cast<double>( n ) ) );
}
int main()
{
std::list<double> numbers;
std::string sInput;
while (getline( std::cin, sInput )) { // added `while` to process all input
std::istringstream parse( sInput );
double tmp;
while ( parse >> tmp )
numbers.push_front( tmp );
}
std::for_each(numbers.begin(), numbers.end(), sqrt_f);
return 0;
}
Then, I'm going to ignore the question you actually asked (about correctness) and just review the (rewritten) code, assuming that it meets the requirements.
void sqrt_f(double n)
{
printf ( "%.4f\n", sqrt( static_cast<double>( n ) ) );
}
I see three things here I don't particularly like:
- Using printf in C++. iostreams are generally preferred (though as we'll see, they are more verbose in this case).
- The static_cast seem to be entirely extraneous (
n
is already a double). Poor separation of concerns. You're combining computation with I/O.
std::list<double> numbers;
I don't see much reason to use std::list
here. You're only inserting at one end, and traversing beginning to end. The primary reason to use std::list
is if you're going to insert/delete somewhere in the middle of the list. Even then it's not necessarily the best choice, but when you only need to insert at one end, it's almost certainly the wrong choice.
std::string sInput;
while (getline( std::cin, sInput )) { // added `while` to process all input
double tmp;
while ( parse >> tmp )
numbers.push_front( tmp );
}
I don't see any real reason to use std::getline
here. getline
is useful primarily when your input has line-oriented formatting, but here it's just a stream of doubles separated by some arbitrary amount of white space. For that, you appear to have added extra complexity without gaining much functionality in return.
I'm also less than excited about the loop while (parse >> tmp)
. It's certainly better than some alternatives, but I'd rather use std::copy
with (in this case) an std::istream_iterator<double>
to copy the values into a collection.
I'm not particularly excited about how you're using the collection here either. First of all, you're reversing the order by using push_front
. At least to me, it seems like it would be easier to understand if you added the numbers to the collection in their original order, then walked through the collection backwards.
std::for_each(numbers.begin(), numbers.end(), sqrt_f);
...and here's the culprit responsible for the poor separation of responsibility in sqrt_f
. It's been something like 15 years since the STL came to C++, and in that time I doubt I've seen as many as half a dozen good uses of std::for_each
. It can be useful, but should generally be your last choice of algorithms, when you just can't find anything else that works better.
I think if I were doing this, I'd use an std::vector
for the storage (we only need to add items to one end, which fits what vector
provides). I'd copy the data directly from std::cin into that vector using std::copy
with a std::istream_iterator<double>
. I'd then use std::transform
with a pair of reverse iterators over the vector, and an ostream_iterator
for the output. Since both the fixed flag and the precision are "sticky", we can get all the output in the correct format by setting the format once before we call std::transform
:
std::cout.setf(std::ios_base::fixed, std::ios_base::floatfield);
std::cout.precision(4);
精彩评论