8
\$\begingroup\$

Challenge

Using the C++ language, have the function PentagonalNumber(num) read num which will be a positive integer and determine how many dots exist in a pentagonal shape around a center dot on the Nth iteration. For example, in the image below you can see that on the first iteration there is only a single dot, on the second iteration there are 6 dots, on the third there are 16 dots, and on the fourth there are 31 dots.

Your program should return the number of dots that exist in the whole pentagon on the Nth iteration.

Sample Test Cases

Input:2
Output:6

Input:5
Output:51

I found it weird that Coderbyte called this a hard challenge. It works, but is this the right approach?

Header File

#ifndef PentagonalNum_hpp
#define PentagonalNum_hpp

#include <stdio.h>

class PentagonalNum
{
public:
    PentagonalNum(int N);
    unsigned long long int num_Of_Dots();
private:
    int iteration;
};
#endif /* PentagonalNum_hpp */   

CPP File

#include "PentagonalNum.hpp"

PentagonalNum::PentagonalNum(int N)
: iteration(N)
{
}

unsigned long long int PentagonalNum::num_Of_Dots()
{
    int result = 1;
    int num = 5;

    for (int i = 1; i < iteration; ++i)
    {
        result += num;
        num += 5;
    }
    return result;
}
\$\endgroup\$

3 Answers 3

15
\$\begingroup\$

It seems a little peculiar to use a class to solve this problem. It's not like there's any need to maintain state. All you're doing, in the end, is making it more difficult to use - instead of:

auto result = pentagonal_number(n);

you have to write:

auto pentagonal_number = PentagonalNum{n};
auto result = num_Of_Dots();

or:

auto result = PentagonalNum{n}.num_Of_Dots();

You could throw out the whole class and just keep the num_Of_Dots() member function, let it take iteration as a parameter, and you'd be done. It would be much easier to use, too.

As for the code itself:

#include <stdio.h>

This is not valid in C++ anymore (though it used to be). You need <cstdio>, or better yet, use actual C++ I/O.

unsigned long long int PentagonalNum::num_Of_Dots()
{
    int result = 1;
    int num = 5;

    for (int i = 1; i < iteration; ++i)
    {
        result += num;
        num += 5;
    }
    return result;
}

It seems like this function can be both noexcept and constexpr (and, if it's going to be a class member, also const). Note that the function requires iteration to be greater than or equal to 1; if you're not checking that here so the function can be noexcept, then you should check it in the constructor.

The big question I have is why result is only an int if you intend to return a unsigned long long. Seems a bit of a waste of all that integer range.

You use some magic numbers and variables with mysterious names (num?). Your code would be clearer and easier to understand with more explicit variable names, like:

constexpr unsigned long long int PentagonalNum(int iteration)
{
    // might want to assert that iteration >= 1

    constexpr auto NUMBER_OF_SIDES = 5uLL;

    auto result = 1uLL; // start with 1 point
    auto points_to_add_on_next_iteration = NUMBER_OF_SIDES;

    for (auto i = 1; i < iteration; ++i)
    {
        result += points_to_add_on_next_iteration;
        points_to_add_on_next_iteration += NUMBER_OF_SIDES;
    }

    return result;
}

As an aside, I suspect the reason this challenge was marked as "hard" is because you are supposed to try and solve it without iterating. In other words, you are supposed to stop and think about the problem and derive an equation for the number of dots at iteration n, rather than just charging in and doing an iterative solution.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks. Can you check my recent post? I don't think anyone will every respond to it. \$\endgroup\$
    – austingae
    Jul 20, 2018 at 2:43
  • \$\begingroup\$ Nowhere does it say you can't use iteration. In fact the wording implies you will use it. \$\endgroup\$
    – Matt Ellen
    Jul 20, 2018 at 23:43
  • \$\begingroup\$ @MattEllen Programming challenges don't generally tell you that you can't brute force the problem; they just penalize you if you do. In this case, it's impractical to detect or penalize so you can get away with it. But where do you think it "implies" you will use iteration in you solution? Just because it uses iteration to explain the problem? It wouldn't be much of a challenge if all you were expected to do is mechanically translate the solution they wrote out for you. \$\endgroup\$
    – indi
    Jul 20, 2018 at 23:53
7
\$\begingroup\$
#ifndef PentagonalNum_hpp
#define PentagonalNum_hpp

Header guards are preprocessor variables and normal naming conventions usually have them as uppercase. Consider adding more differentiators that reduce the probability a name collision occurs (GUID, author, project physical path, project logical path, dates, etc).

#include <stdio.h>

Use the C++ version of libraries instead of the C version of libraries. <cstdio> instead of "stdio.h". The standard only promises that symbol names will be in the std namespace and possibly be aliased to the global namespace.

Why does your header require stdio.h? Only include what you need.

class PentagonalNum

Why an object instead of a function?

Symbols you declare/define should be in its own namespace. Do not pollute the global namespace of those that use your headers.

    PentagonalNum(int N);
    unsigned long long int num_Of_Dots();

Rather than recomputing the number of dots, perhaps you should precalculate the result and just return it.

const and noexcept qualify functions if applicable.

explicit qualify your constructor to disable implicit conversions.

unsigned long long int PentagonalNum::num_Of_Dots()
{
    int result = 1;
    int num = 5;

    for (int i = 1; i < iteration; ++i)
    {
        result += num;
        num += 5;
    }
    return result;
}

What happens if iteration is negative or zero? Is the result really 1?

Have you considered a mathematical approach? Working out the first five values, we have a sequence of 1, 6, 16, 31, 51. Plugging that sequence into OEIS returns "Centered Pentagonal Numbers: (5n^2+5n+2)/2". If you plan to support large values of \$num\$, consider the mathematical approach.

\$\endgroup\$
0
2
\$\begingroup\$

Let's say we have different "layers": 1, 2, 3, 4, ...

with 1 layer, you have 1 dot with 2 layers, you have 1 + 5 dots with 3 layers, you have 1 + 5 + 10 dots etc

You can solve this mathematically.

After layer 1, you have 5 segments with equal amount of dots:

on layer 1 there are 5 segments with 0 dots on layer 2 there are 5 segments with 1 dot on layer 3 there are 5 segments with 3 dots (1 + 2) on layer 4 there are 5 segments with 6 dots (1 + 2 + 3) etc...

On top of these 5 segments, we have the 1 center dot. 1 + 2 + 3 + .. + n can be rewritten as n x (n + 1) / 2.

n = layer number (L) - 1

so per segment we have (L-1) x ((L-1) + 1) / 2 = Lx(L-1)/2

With 5 centers and a center dot this gives the formula:

1 + (5/2 x L x (L-1))

so you can write your function like (pseudo language):

long calculatePentagonDots(int nrOfLayers)
{
    return 1 + (5/2 x nrOfLayers x (nrOfLayers - 1));
}

and no need for recursion or loops.

\$\endgroup\$
2
  • \$\begingroup\$ 5/2 as integral numbers won't do what you want it to do. Bringing floating point arithmetic in will complicate things. \$\endgroup\$
    – Summer
    Jul 20, 2018 at 14:42
  • \$\begingroup\$ Using return 1 + 5*(nrOfLayers * (nrOfLayers - 1) / 2); you'll prevent division errors (but not integer overflows). \$\endgroup\$
    – Bob__
    Jul 20, 2018 at 14:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.