patterncppMinor
"Jolly Jumper" challenge
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:
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.
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
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 10main.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:
Issues I see include:
In summary, I suggest:
#ifndef __JOLLY_JUMPER_H_
#define __JOLLY_JUMPER_H_
#include
#include
#include
#include
class JollyJumper
{
private:
public:
JollyJumper();
bool IsJolly(std::string input);
};
#endifIssues I see include:
- Pay attention to const correctness. The parameter should be
const, and the method should beconst.
- 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 aconst 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
privatekeyword 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);
};
#endifstruct JollyJumper
{
static bool IsJolly(const std::vector &input);
};Context
StackExchange Code Review Q#62665, answer score: 4
Revisions (0)
No revisions yet.