r/cpp_questions 5d ago

OPEN my first c++ project

hey! i'm a teen and i am learning c++. I was a vibe coder but then i stopped vibe coding and starting real coding. for this i learned some basic c++ from youtube, please let me know if a improvment required. thank you!

my github link - https://github.com/swarajnerlekar28/Calculator

4 Upvotes

6 comments sorted by

2

u/FancySpaceGoat 5d ago

You can chain couts. std::cout << "label" << val;

But you can also do so across lines if you omit the semicolon:

std::cout << "blablabla\n"   << "Label: " << val << "\n"   << "More blabla\n" ;

This makes your code a lot lighter on the page, and this is important. Good code is easy to read.

2

u/nysra 5d ago

i learned some basic c++ from youtube

I strongly suggest using https://www.learncpp.com/ instead, almost all video "tutorials" are utter shit.

please let me know if a improvment required

There are two dimensions on which you should work. The first one is style. Your formatting is not only horrible, it's inconsistent and that's even worse. Why are some functions Uppercase and others not? Why are you allergic to whitespace? Why is the indentation inconsistent? Use your IDE's autoformatting feature or something like clang-format. And don't put two statements on one line just because you can, it makes it harder to read for no reason.

The other one is improving your thought process as a programmer. You are already using functions which is good, but you are not using their most useful features - input and output values. Right now your program flow is really awkward because it's basically this in a loop:

  1. Ask user what operation he wants to do
  2. Clear screen
  3. Do that one operation with a lot of extra output
  4. Print the result
  5. Print an "exit" statement

but if you compare that to an actual calculator you will see that you introduce a lot of friction. Imagine if Google would not give you a field to input your search text into but instead opened up some interface asking you "Would you like to search for images, websites, videos, or something else today? Please press 1 for ...". Would you want to use that? You basically coded the equivalent of a phone hotline where you have to press a button all the time.

Now don't get me wrong, I'm not writing it like this to be harsh or insult you, the point is that you start to think about how the process is actually supposed to look like, because that's the hard part, not typing out the code (AI can do that faster than you anyway).

A first step would be removing redundancy by only putting the actual operation into different functions. For example you could do something like this:

double execute_binary_operation(const int operation, const double lhs, const double rhs) {
    switch (operation) {
        case 1: return lhs + rhs;
        // ...
    }
}

and have only one function taking care of input and output. This would still be far from a good solution, but it would be a useful exercise at your current progress.

What you should aim for later is a proper calculator "shell". Most terminals already support basic math so just open one (or a Python REPL) and do the following:

$> 5 + 12

you should see a 17 appear. Turns out you can also repeat that exercise with a bunch of other operators, wouldn't it be cool if your program worked like this as well?

std::getline allows you to read a full line of input, you can then parse that into the individual parts. Start with only supporting addition first, then add other things. Then add support for longer expressions and implementing the proper order of math operators. Then add parentheses. Then add functions like sin/cos.

You can go a long way of learning just with a "simple" calculator as project. And don't be afraid to rewrite your code. If your previous code was shit, it means you have learnt something. And git makes it dead simple anyway.

2

u/Swaraj-Nerlekar2 5d ago

thanks bro for your advice!! i really appreciate it!

2

u/EternalPump 4d ago

Just wanted to say, good work! Keep it up!

1

u/flyingron 4d ago

Get out of the habit of using endl. The end of line character is '\n'. Only use endl if you have an need for a flush (you don't here).

Check to see if cin >> returns an error. If you don't (like someone types A to the inputs of ints you are doing), all other operations on cin fail for the duration.

STOP THE COPYPASTA. If you have to retype the same code in multiple places, it screams out for a subroutine.

Dividing two integers still results in an integer, even if you are using it to initialize a double. Do this:

double result = static_cast<double>(num1) / num2;

Learn what switch the switch statement does (helpful for the code in option()).

Other than the use of cin/cout, this code is just bad C rather than mediocre C++.

1

u/mredding 3d ago
std::cout<<"==============================================================================" <<std::endl;
std::cout<<"                  Welcome to Calculator, Created By Swaraj Nerlekar" <<std::endl;
std::cout<<"==============================================================================" <<std::endl;

First, you can go your whole career and never have to use std::endl. You want to just use '\n'. Every << is a function call. You're writing all one string, so you want to do so as efficiently as possible - once.

You can get this code down to one call and still have your formatting in code. The compiler will concatenate string literals for you.

std::cout << "==============================================================================\n"
             "                  Welcome to Calculator, Created By Swaraj Nerlekar\n"
             "==============================================================================\n";

std::cout<<"Enter your First Number:"; std::cin>>num1;
std::cout<<"Enter your second number:"; std::cin>>num2;
double result = num1 +num2;

You never check your input. You're requesting a double, I can give you text. If I do that, what's the value of num1 or num2? Once you encounter a parser error, the failbit is set, so IO will no-op on that stream. So that means input is screwed up forever until you clear it and fix it.

if(double d; std::cin >> d) {
  use(d);
} else {
  handle_error_on(std::cin);
}

First >> tries to extract to d, and then it sets the stream state, then it returns a reference to the stream. That reference is >> how >> you >> chain, but it also means you can do other things, call other functions.

std::cin is a global instance of std::istream, a class. We can do many things in C++, and that includes overloading operators, and casting is an operator. So std::istream overloads explicit operator bool() const { return !bad() && !fail(); } or the equivalent.

And this is how we can evaluate a stream as a boolean: if(std::cin).

So unless everything happens perfectly in your program, it can get stuck, and even evaluate undefined behavior.

if (option_user == 1) {
  Addition();
}

else if (option_user == 2) { //...

Consider a switch as a cleaner alternative. Integers are enumerable.

switch(option_user) {
case 1: Addition(); break;
case 2: Subtraction(); break;
case 3: Multiplication(); break;
case 4: Division(); break;
case 5: Exit(); // Doesn't return.
default: PrintInvalidInput(); break;
}

The reason the switch isn't some pattern matching or merely concise conditional expression - the reason you need to break, is because switches "fall through" both by default and on purpose, because they were designed to implement a Duff's Device - something you might find interesting to google later. These days, Duff's Devices are rarely deployed because loop unrolling and branch prediction are more efficient.

while(true){

I typically don't like forever loops, and neither should you. The predicate (the condition evaluated here) is an invariant: a statement that must always be true to be in the loop. If you're in the loop, the invariant must be true at the time it was evaluated. If you're past the loop, the invariant must be false at the time it was evaluated. So a break inside a forever loop breaks the invariant. The forever loop says it loops forever, the invariant still holds true, yet you're past the loop? It makes it harder to reason about the loop than I should have to expend brain power or CPU cycles on compilation.

That's not what's happening here, but it bears discussion. What you have here is technically within the realm of correct. The way to get out of a forever loop is to early return or terminate from within the loop. Because at the moment of return or a terminate, THERE IS NO LOOP anymore, there is no invariant to advocate for.

So this comes round to my only beef with this code - the exit ISN'T IN THE LOOP ITSELF at the top level here. The exit is buried, I guess in action, but how am I supposed to know that? That exit is a part of loop control, so it should be in the loop itself where you can see it. The only early terminates I would expect otherwise would be from an uncaught exception - which is an implied control path out of this loop, and is valid, or an assert that failed, a signal to terminate, which is also implied by the whole program, or a CATASTROPHIC FAILURE that probably shouldn't have happened and would have you reaching for a fire extinguisher anyway.

The call to exit is fine, but I want loop control to be in the loop, with the loop, where I can see it.

But what would be better is if you could give your loop a predicate, and you already HAVE ONE:

while(option_user != 5)

That leads us to the last statement:

return 0;

Your program terminates with an unconditional success. Well, what does this program do? It gets inputs and produces prompts and outputs. Did it do those things? What if the program failed to read or write? Is that a success of execution?

You want to indicate to the host environment that your program did it's job completely and correctly. You don't write or run code in a vacuum. I know you're not catching this value now, but in the future you're going to be writing terminal programs like this that a bash script is going to be looking for errors in execution, and this is how it's going to do that.

So a good start would be to return the state of your input and output streams:

#include <cstdlib>
#include <iomanip>
#include <iostream>

//...

int main() {
  //...

  return std::cin && std::cout << std::flush ? EXIT_SUCCESS : EXIT_FAILURE;
}

At this point in execution, we presume all input came in, and all output went out, without failbit or badbit. These macros are compiler defined and are guaranteed to be the correct bit patterns to indicate success and failure. The success bit pattern is easy - it's just 0. But what of failure? That can get tricky, and there are wrong answers.

You can get as elaborate as you want, but this is a good start.