HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

"Jolly Jumper" challenge

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
jumperjollychallenge

Problem

I'm a C#/Java full-time developer who is trying to pick up C++ purely for educational reasons. I've tried to solve a pretty simple ACM ICPC problem and would love to hear all kinds of criticisms on how to write acceptable modern C++ code. (Seriously, tell me that my code is a joke, as long as you can tell me how to make it better.)

Specifically, I'd like to hear comments on the following aspects:

  • Does the code follow typical C++ coding convention?



  • Is it cross platform? Especially regarding how I am handling strings.



  • Is it C++11 compliant? Am I making a reasonable use of it?



  • Is the code readable?



You're also welcome to comment on my algorithm on solving the problem, although that's less of a concern for me at the moment because the primary purpose for me is to learn how to properly use C++.

Full source (VS2012)

The challenge is to determine whether each line of input contains a "jolly jumper" sequence:


A sequence of n > 0 integers is called a jolly jumper if the absolute values of the difference between successive elements take on all the values 1 through n-1. For instance,



1 4 2 3



is a jolly jumper, because the absolutes differences are 3, 2, and 1 respectively.

Below is a test input you can try running against the program. The program works as intended as far as I can tell.

4 1 4 2 3
5 1 4 2 -1 6
10 1 2 3 4 5 6 7 8 9 10
10 1 2 4 7 11 16 22 29 37 46
10 -1 -2 -4 -7 -11 -16 -22 -29 -37 -46
10 -1 -1 -4 -7 -11 -16 -22 -29 -37 -46
1 1
2 1 2
2 2 1
4 0 4 2 3
4 1 3 2 4
1 2
6 1 4 3 7 5 10


main.cpp

```
#if defined(WIN32) || defined(_WIN32) || defined(__WIN32) && !defined(__CYGWIN__)
#define WIN32_DEF 1
#else
#define WIN32_DEF 0
#endif

#if WIN32_DEF
#include "win32helper.h"
#endif

#include "jolly_jumper.h"

#include
#include
#include

using std::cin;
using std::cout;
using std::endl;
using std::string;
using std::ifstream;

int main(int argc, char argv[], char envp[]) {
#if _DEBUG && WIN32_DEF
//Vi

Solution

I'll just focus on your jolly_jumper.h:

#ifndef __JOLLY_JUMPER_H_ 
#define __JOLLY_JUMPER_H_

#include 
#include 
#include 
#include 

class JollyJumper
{
private:

public:
    JollyJumper();
    bool IsJolly(std::string input);
};

#endif


Issues I see include:

  • Pay attention to const correctness. The parameter should be const, and the method should be const.



  • The method is going to be doing too much: it needs to parse the input string as a vector, then run its algorithm. The IsJolly() method should take a vector (as a const std::vector&).



  • Your class is "degenerate". There's no state. Your constructor does nothing. You might as well write it as a standalone function, or perhaps a class that contains a static function.



  • The private keyword is just noise.



In summary, I suggest:

struct JollyJumper
{
    static bool IsJolly(const std::vector &input);
};

Code Snippets

#ifndef __JOLLY_JUMPER_H_ 
#define __JOLLY_JUMPER_H_

#include <iostream>
#include <string>
#include <vector>
#include <set>

class JollyJumper
{
private:

public:
    JollyJumper();
    bool IsJolly(std::string input);
};

#endif
struct JollyJumper
{
    static bool IsJolly(const std::vector &input);
};

Context

StackExchange Code Review Q#62665, answer score: 4

Revisions (0)

No revisions yet.