I was attempting the Fencing Problem of USACO 2015 Bronze. http://www.usaco.org/index.php?page=viewproblem2&cpid=567

This problem basically asks you to find the total length of the fence painted given that two individuals (a cow and farmer) paint fences between a given interval on the number line (the fence is the number line over here). The painting can overlap, we need to find the total length of fence finally covered in paint. This is my code but I don't get the required result even though I am not able to find a logical error with in my solution, probably there is some error in the syntax of my code.

CODE:

#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;
}

INPUT:

7 10 
4 8

OUTPUT: I get extremely large different numbers each time (For example: 1063908272)

I am also open to any suggestions to improve my code if it is redundant, I am a beginner.

CodePudding user response：

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;
}

CodePudding user response：

1. You need file IO paint.in/out
2. Your algo is wrong. I suggest you try inclusion/exclusion principle, i.e. sum up the two intervals and subtract the common part.