Why my code is not working for particular test case

Question

#include<bits/stdc++.h>

using namespace std;


class Box{
  private:
  int l,b,h;  

public:
Box(){
    l=0;
    b=0;
    h=0;
}
Box(int a,int d,int c){
        l=a;
    b=d;
    h=c;
    
}
Box(Box& source){
    l=source.l;
    b=source.b;
    h=source.h;  
}
int getLength(){
    return l;
}
int getBreadth (){
    return b;
}
int getHeight (){
    return h;
};

long long CalculateVolume(){
     return this->l*this->b*this->h;
};

friend bool operator<(Box & s1,Box &source){
    return (s1.l*s1.b*s1.h)<(source.l*source.b*source.h);
}

friend ostream& operator<<(ostream& out,const Box& B){
    out<<B.l<<" "<<B.b<<" "<<B.h;
    return out ;
}


};



void check2()
{
    int n;
    cin>>n;
    Box temp;
    for(int i=0;i<n;i++)
    {
        int type;
        cin>>type;
        if(type ==1)
        {
            cout<<temp<<endl;
        }
        if(type == 2)
        {
            int l,b,h;
            cin>>l>>b>>h;
            Box NewBox(l,b,h);
            temp=NewBox;
            cout<<temp<<endl;
        }
        if(type==3)
        {
            int l,b,h;
            cin>>l>>b>>h;
            Box NewBox(l,b,h);
            if(NewBox<temp)
            {
                cout<<"Lesser\n";
            }
            else
            {
                cout<<"Greater\n";
            }
        }
        if(type==4)
        {
            cout<<temp.CalculateVolume()<<endl;
        }
        if(type==5)
        {
            Box NewBox(temp);
            cout<<NewBox<<endl;
        }

    }
}

int main()
{
    check2();
}

when I input this.

6
2 1039 3749 8473
4
3 1456 3836 283
3 729 3749 272
2 4839 283 273
4

Output should be .. What I get

1039 3749 8473
33004122803                      here  -1355615565
Greater
Lesser                           Greater
4839 283 273
373856301

all other output is correct I dont know why I am getting this type of bug in my code .

Please read [Why should I not #include ?](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h) and [Why is "using namespace std;" considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) — Jesper Juhl, Jul 19 '23 at 12:19
Do you know what the maximum value is that an `int` can hold and what happens when a calculation exceeds it? — mch, Jul 19 '23 at 12:19
This should help: [What is a debugger and how can it help me diagnose problems?](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems) — Jesper Juhl, Jul 19 '23 at 12:20
`33004122803` is larger than an `int` on many systems. Try `int64_t` here: [https://en.cppreference.com/w/cpp/types/integer](https://en.cppreference.com/w/cpp/types/integer) — drescherjm, Jul 19 '23 at 12:23
Side note: You should get used to implementing the constructors initialiser list (not to be confused with `std::initializer_list`): `Box() : l(0), b(0), h(0)` – this prefers direct initialisation by value over default initialisation + assignment – which for complex types can come with significant performance overhead; even more important: some types *only* can be initialised that way (`const` members, references, non-default-constructible types, ...). — Aconcagua, Jul 19 '23 at 12:30
`Box(int a,int d,int c){ l=a; b=d; h=c; }` - Don't assign members in the constructor body unless you have to. Prefer to use the member initialization list. — Jesper Juhl, Jul 19 '23 at 12:31
Do negative dimensions have any meaning in your scenario? If not (which I assume), you might better reflect this fact by using `unsigned int` for the dimensions. — Aconcagua, Jul 19 '23 at 12:31
https://stackoverflow.com/questions/29235436/c-integer-overflow — pptaszni, Jul 19 '23 at 12:34
You are mixing naming conventions for your functions (`camelCase` for your getters, `PascalCase` for volume calculation), you should decide for one of and keep that consistently – the former being more common, the latter mostly in environments dominated by Microsoft, I'd recommend the former as long as you have the choice. — Aconcagua, Jul 19 '23 at 12:35
There's an alternative convention for getters and setters, instead of redundant get/set prefixes just leaving these ones out and if both exist distinguish the *overloads* by signature, i.e. `unsigned int length(); unsigned int breadth(); unsigned int height(); /* and if setters exist: */ void length(unsigned int value); void breadth(unsigned int value); void height(unsigned int value);` – just for the case that you might like (I personally prefer…). In given case I'd even prefer `unsigned long long volume();` — Aconcagua, Jul 19 '23 at 12:38
`Box(Box& source)` -- There is no need for this function. The `Box` class has correct copy semantics already, optimized and with no bugs. — PaulMcKenzie, Jul 19 '23 at 12:40
... and if you still want to have explicitly you can simply default it: `Box(Box const& source) = default;` – note, too: introducing `const` correctness! And note yet another time: Having it explicitly eliminates the *move constructor* (`Box(Box&& source)`) – which you admittedly would't need in given case, but in other scenarios you'd drop a (free!) optimisation opportunity (unless you default it explicitly as well, that is). — Aconcagua, Jul 19 '23 at 12:42
You only provide `less` comparison semantics – if you have C++20 available you might instead implement the space-ship operator: `std::weak_ordering operator<=>(Box const& x, Box const& y) { return x.volume() <=> y.volume(); }`, you then get all six other comparisons (==, !=, <, <=, >, >=) for free with. — Aconcagua, Jul 19 '23 at 12:51
Just a minor issue, but if you output single characters then using character literals instead of string literals is more efficient: `std::cout << l << ' ' << b << ' ' << h`. — Aconcagua, Jul 19 '23 at 12:53
Please [edit] your code to reduce it to a [mcve] of your problem. Your current code includes much that is peripheral to your problem - a minimal sample normally looks similar to a good unit test: only performing one task, with input values specified for reproducibility. — Toby Speight, Jul 19 '23 at 13:02
Sorry, but Stackoverflow is not really the best place to ask questions about specific coding puzzles from other web sites. Experienced Stackoverflow users generally have no interest in random coding puzzles. You can use the forums on those web sites to ask any questions about their coding puzzles, to people who spend a lot of time doing them. If you can [edit] your question and focus it on a specific C++ subject matter or topic, not related to any specific coding puzzle, then you're welcome to ask it. — Sam Varshavchik, Jul 19 '23 at 13:22
It may be required by LeetCode problem, but I find using integers for computing a volume very odd. In real life, distances, and therefore derivatives like surface or volume, are never integers (why on Earth would my bottle height be an integer number of meters?). So `double` is the type to use for all real quantities, and `double` don't suffer from a limited range like `int`. — prapin, Jul 19 '23 at 14:12
Height, width, etc... as an integer might make sense if the measurement increment is small enough. For example, the height of the bottle in millimeters will be sufficient for many real-world cases. See [Fixed Point](https://en.wikipedia.org/wiki/Fixed-point_arithmetic). — user4581301, Jul 19 '23 at 14:21

score 4 · Accepted Answer · answered Jul 19 '23 at 12:29

You return a long long when calculating the volume, but the compiler sees this too late, so you still get an overrun. Promoting one value in the calculation will make the compiler do the promotion of the calculation

long long CalculateVolume(){
     return static_cast<long long>(l)*b*h;
};

Also, use CalculateVolume in operator< so the cast only has to be done once.

Why my code is not working for particular test case

1 Answers1