0
#include<iostream>
#include<stack>
#include<cmath>
using namespace std;

int Narsic(stack<int> stk)
{
    int x=0;
    int temp=0;
    int val=0;
    int power_count = stk.size();

    while(! stk.empty())
        {
            x = stk.top();
            stk.pop();
            temp = pow(x,power_count);
            val = val + temp;

        }
        cout<<val;

}


int main()
{
    int num,indic;
    num = 1652;
    stack<int> numstack;

    while(num>0)
        {
            indic = num%10;
            num = num/10;
            numstack.push(indic);
        }

    Narsic(numstack);





    return 0;
}

Basically this program is to find the Narcissistic value of a given integer, my codes can be successfully executed and it is correct, but somehow, i think , maybe it is a bit lengthy, so the purpose of posting this problem is, whether anyone can give me suggestions on how to improve the codes above? Sorry , I'm a beginner and just started learning C++. I hope this community won't get furious against by sucky codes XD. (Oh yeah, i do search for some codes online and implement them partially here, so if anyone saw similar codes, please be aware that i have made changes, sorry if this irritates you.)

Wog12222
  • 35
  • 1
  • 7
  • As soon as you mix integers and floats (`pow`) it's going to go wrong. – Richard Critten May 25 '21 at 13:48
  • got it, should i change all of them to float? – Wog12222 May 25 '21 at 13:49
  • are you saying that if during i am writing a huge project or many lines of codes and i mix integers with float, there's a high possibility that i will definitely have bugs ? – Wog12222 May 25 '21 at 13:52
  • Nope always use integers (for this type of problem) unless converting to a string first makes the algorithm easier. And have a read of https://stackoverflow.com/questions/588004/is-floating-point-math-broken – Richard Critten May 25 '21 at 13:54
  • Its not recommended to use `using namespace std;` better to add `std::` anywhere it needed than shoot youself in the foot by using some already used in `std` name. Also, in C++ its preferred to declare variables as late as possible, i.e. declare `x`, `tmp` and `indic` when you first assign them. – sklott May 25 '21 at 13:54
  • Thanks, @RichardCritten, thats a useful source ! – Wog12222 May 25 '21 at 14:01
  • @sklott, Thanks for the advice, although i don't understand why is that, but I will google deeper for details on it. – Wog12222 May 25 '21 at 14:02
  • I keep [Why is “using namespace std;” considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice) in my bookmarks list, not because I need it at this point, but because it saves time searching for it. – user4581301 May 25 '21 at 14:17
  • @user4581301, thanks for that, it is informative – Wog12222 May 25 '21 at 15:03

1 Answers1

1

It looks like you're largely asking about style. I'm going to take your code and edit it to be more in keeping with what I would do, then I'll comment below.

#include <iostream>
#include <stack>
#include <cmath>

using std::cout;
using std::endl;
using std::stack;

/**
 * Return the Narcissistic value of the digits stored in the stack.
 * A Narcissistic value is (insert description).
 */
int narsic(stack<int> stk)
{
    int result = 0;
    int power_count = stk.size();

    while (! stk.empty() ) {
        int digit = stk.top();
        stk.pop();
        result += pow(digit, power_count);
    }

    return result;
}

/**
 * Entry point.
 */
int main() {
    int num;
    num = 1652;
    stack<int> numstack;

    while (num>0)
        {
            int indic = num % 10;
            num /= 10;
            numstack.push(indic);
        }

    int result = narsic(numstack);
    cout << "Result: " << result << endl;

    return 0;
}

I use Java's naming conventions. Class names begin with an upper case letter. Variables and methods begin with lower case letters. This isn't necessarily the C++ way of doing it, but I programmed Java a long time, and it's what I prefer. You'll want to find a style for your academic work, and then use whatever your future employer uses.

I prefer more whitespace to make code readable -- especially for myself. I'm nearly 60 years old, with old eyes, and as code runs all together, it gets harder and harder to read. I've added whitespace here and there.

A blanket using namespace std is considered dangerous, but I don't like sticking std:: all over the place, so I tend to add specific using statements, although not too many.

You were missing a return statement in narsic().

And you had an unusual indentation style. Old-school C/C++ is either 4 spaces or use a tab, but set your editor to 4-space tabs. You indented the braces and then indented again. That would be odd. Some people put the open brace on the same line (I moved them) and some on the next line the way you did. I actually will put them on the next line if it makes the code more readable -- like if there's an obnoxiously long and complicated if-clause.

I moved variable declaration to just as the variables were about to used. This is better for a variety of reasons, including one less initialization plus makes your code smaller. Plus, it keeps scoping rules much tighter.

I switched a num = num / 10 to num /= 10. Similar thing with result +=.

I don't like the variable name val. I changed to result. I think I might have made one or two other variables more descriptive, too.

I added function comments and I did it in the doxygen style. Commenting each method means tools can auto-generate documentation. It also helps break code up a little bit better visually. For comments, it's important to provide information not in the code. The comment for main is trivial, but having it still helps if you actually use a documentation generator -and- provides a visual break.

A change I did NOT make but would normally do: I prefer main to be the first method in my files (at the top), so I would have made a forward reference to narsic() and moved it below main. I didn't do that just because I wanted to keep the code in the same order you had it.

I moved the cout statement and included an endl.

Joseph Larson
  • 8,530
  • 1
  • 19
  • 36
  • wow! thats amazing, thanks for that advice, i found your code more readable than mine, will take your suggestions to put into my codes. – Wog12222 May 26 '21 at 05:39