1. length might not be initialized
Like @AlanBirtles pointed out in the comments, length would not be initialized in case b <= d which would lead to undefined behaviour.
This could be fixed by moving the assignment of length outside the else statement:
godbolt example
#include <iostream>
#include <string>
using namespace std;
int main() {
int lowerlimit, upperlimit, a, b, c, d, length;
cin >> a >> b >> c >> d;
if (a < c) {
lowerlimit = a;
}else{
lowerlimit = c;
}
if (b > d) {
upperlimit = b;
}else{
upperlimit = d;
}
length = upperlimit - lowerlimit;
cout << length;
return 0;
}
After this your program will already produce the expected outcome (assuming you get the input via stdin and need to output to stdout)
2. Do you need to read the input file / write to the output file?
In the problem statement link you posted there's an explicit mention of an input file (paint.in) and output file (paint.out) - so it might be that you're expected to read your input from that file instead of from stdout (like @Qingchuan Zhang mentioned in his answer)
In case you don't get any stdin input, your a, b, c & d would be left uninitialized und you'd get undefined behaviour once you try to use them.
So in case you're actually required to read the input from a file, you'd have to change your code for that, e.g.:
int main() {
int lowerlimit, upperlimit, a, b, c, d, length;
std::ifstream input("paint.in");
input >> a >> b >> c >> d;
if (a < c) {
lowerlimit = a;
}else{
lowerlimit = c;
}
if (b > d) {
upperlimit = b;
}else{
upperlimit = d;
}
length = upperlimit - lowerlimit;
std::ofstream output("paint.out");
output << length;
return 0;
}
3. Potential improvements to your code
This is just a small list of things that i'd personally change, YMMV.
- The
if statements for lowerlimit / upperlimit effectively do what std::min / std::max would do, respectively. So i'd go with the standard version, e.g.:
int a, b, c, d;
cin >> a >> b >> c >> d;
cout << max(b, d) - min(a, c);
using namespace std; - Including the entire std namespace might be convenient, but it's rather easy to create ambiguity problems with it - which is why it's recommended to never include namespaces as a whole (see isocpp coding standards / Why is "using namespace std;" considered bad practice? )
Although it doesn't matter that much for a short piece of code, it's a good habit to get into typing std:: out every time.
So we end up with this:
#include <iostream>
#include <string>
int main() {
int a, b, c, d;
std::cin >> a >> b >> c >> d;
std::cout << std::max(b, d) - std::min(a, c);
return 0;
}
- Variable names: I do agree with @digito_evo's comment that in general more descriptive variable names would be nice - but in the question you've linked they're also called
a, b, c, d - so in this case i'd probably leave them as they are to make them consistent with the question.
- Error checking. Whenever you do something that might fail (reading from stdin, writing to file, etc...) it's generally a good idea to check if what you tried was actually successful (in this case
a, b, c, d might be left uninitialized in case the input operation fails)
In this case you can just check if std::ios_base::iostate::failbit is set on the stream, which would indicate that one of the int's failed to be properly parsed. (in this case i'm using std::ios::operator bool() to test it):
int a, b, c, d;
std::cin >> a >> b >> c >> d;
if(!std::cin) {
std::cout << "Invalid input!";
return 1;
}
std::cout << std::max(b, d) - std::min(a, c);
return 0;
So after incorporating all those things this is what i ended up with:
godbolt example
#include <iostream>
#include <string>
int main() {
int a, b, c, d;
std::cin >> a >> b >> c >> d;
if(!std::cin) {
std::cout << "Invalid input!";
return 1;
}
std::cout << std::max(b, d) - std::min(a, c);
return 0;
}
