I've been writing code for some time but never had the need (or opportunity) to write C/C++ code. Did a small project to learn some C++ and would appreciate feedback on code, testing, project structure and building.
Project Layout
.
├── CMakeLists.txt
├── src
│ ├── personnummer.cpp
│ └── personnummer.hpp
└── test
├── catch.hpp
└── unittest.cpp
CMakeLists.txt
cmake_minimum_required(VERSION 3.0)
project(cpp-personnummer)
set(CMAKE_CXX_STANDARD 11)
set(SOURCE_FILES src/personnummer.cpp)
add_library(Personnummer ${SOURCE_FILES})
target_link_libraries(Personnummer -lpthread)
add_executable(unittest test/unittest.cpp)
target_link_libraries(unittest Personnummer)
src/personnummer.hpp
#include <ctime>
#include <iostream>
#include <vector>
namespace Personnummer {
int stoi_or_fallback(const std::string &maybe_digit, int fallback);
bool valid_date(int year, int month, int day);
int checksum(std::tm date, int number);
void collect_digits(std::vector<int> &digits, int num);
void collect_digits_pad_zero(std::vector<int> &digits, int num, int min_len);
struct Personnummer {
std::tm date;
int number;
int control;
char divider[1];
bool is_valid_date() {
return valid_date(date.tm_year, date.tm_mon, date.tm_mday);
};
bool is_valid_luhn() { return checksum(date, number) == control; };
bool valid() { return is_valid_date() && is_valid_luhn(); };
};
bool from_string(const std::string &pnr, Personnummer &personnummer);
} // namespace Personnummer
src/personnummer.cpp
#include "personnummer.hpp"
#include <regex>
namespace Personnummer {
bool from_string(const std::string &pnr, Personnummer &personnummer) {
std::regex pnr_regex(
"^(\\d{2})?(\\d{2})(\\d{2})(\\d{2})([-|+]?)?(\\d{3})(\\d?)$");
std::smatch matches;
int century, year;
if (std::regex_search(pnr, matches, pnr_regex)) {
century = stoi_or_fallback(matches.str(1), 19);
year = stoi_or_fallback(matches.str(2), 0);
personnummer.date.tm_year = century * 100 + year;
personnummer.date.tm_mon = stoi_or_fallback(matches.str(3), 0);
personnummer.date.tm_mday = stoi_or_fallback(matches.str(4), 0);
personnummer.number = stoi_or_fallback(matches.str(6), 0);
personnummer.control = stoi_or_fallback(matches.str(7), 0);
personnummer.divider[0] = *matches.str(5).c_str();
} else {
return false;
}
return true;
}
int stoi_or_fallback(const std::string &maybe_digit, int fallback) {
try {
return std::stoi(maybe_digit);
} catch (...) {
return fallback;
}
}
bool valid_date(int year, int month, int day) {
if (month < 1 || month > 12)
return false;
if (day < 1 || day > 31)
return false;
if (day > 30) {
switch (month) {
case 2:
case 4:
case 6:
case 9:
case 11:
return false;
}
}
if (month == 2 && day > 28) {
bool is_leap_year = year % 400 == 0 || (year % 100 != 0 && year % 4 == 0);
if (day != 29 || !is_leap_year) {
return false;
}
}
return true;
}
int checksum(std::tm date, int number) {
std::vector<int> digits;
collect_digits_pad_zero(digits, date.tm_year % 100, 2);
collect_digits_pad_zero(digits, date.tm_mon, 2);
collect_digits_pad_zero(digits, date.tm_mday, 2);
collect_digits_pad_zero(digits, number, 3);
int sum = 0;
for (int i = 0; i < digits.size(); i++) {
int temp = digits.at(i);
if (i % 2 == 0) {
temp *= 2;
if (temp > 9)
temp -= 9;
}
sum += temp;
}
return 10 - (sum % 10);
}
void collect_digits(std::vector<int> &digits, int num) {
if (num > 9)
collect_digits(digits, num / 10);
digits.push_back(num % 10);
}
void collect_digits_pad_zero(std::vector<int> &digits, int num, int min_len) {
// New vector for this section.
std::vector<int> section_digits;
// Collect the digits from given number.
collect_digits(section_digits, num);
// Add the potential padded zeroes.
int missing_digits = min_len - section_digits.size();
for (int i = 0; i < missing_digits; i++) {
section_digits.insert(section_digits.begin(), 0);
}
// Add the padded section to final vector.
digits.insert(digits.end(), section_digits.begin(), section_digits.end());
}
} // namespace Personnummer
test/unittest.cpp
#define CATCH_CONFIG_MAIN
#include "../src/personnummer.hpp"
#include "catch.hpp"
TEST_CASE("Valid date", "[date]") {
std::vector<std::vector<int>> valid_dates = {
{1990, 1, 1}, {1990, 1, 31},
{1990, 2, 28}, {2016, 2, 29}, // 2016 is leap year
{2020, 4, 30},
};
std::vector<std::vector<int>> invalid_dates = {
{1990, 13, 1},
{1990, 1, 32},
{2017, 2, 29}, // 2017 is not leap year
{2020, 4, 31},
};
for (int i = 0; i < valid_dates.size(); i++) {
std::vector<int> test_case = valid_dates[i];
std::stringstream case_title;
case_title << "Testinv VALID: Y=" << test_case[0] << ", M=" << test_case[1]
<< ", D=" << test_case[2];
SECTION(case_title.str()) {
REQUIRE(
Personnummer::valid_date(test_case[0], test_case[1], test_case[2]));
}
}
for (int i = 0; i < invalid_dates.size(); i++) {
std::vector<int> test_case = invalid_dates[i];
std::stringstream case_title;
case_title << "Testinv INVALID: Y=" << test_case[0]
<< ", M=" << test_case[1] << ", D=" << test_case[2];
SECTION(case_title.str()) {
REQUIRE(
!Personnummer::valid_date(test_case[0], test_case[1], test_case[2]));
}
}
}
TEST_CASE("Valid personal number", "[pnr]") {
Personnummer::Personnummer p;
std::vector<std::string> valid = {
"6403273813", "510818-9167", "19900101-0017", "19130401+2931",
"196408233234", "0001010107", "000101-0107",
};
std::vector<std::string> invalid = {
"640327-381",
"6403273814",
"640327-3814",
};
for (int i = 0; i < valid.size(); i++) {
std::stringstream case_title;
case_title << "Testing VALID: " << valid[i];
SECTION(case_title.str()) {
REQUIRE(Personnummer::from_string(valid[i], p));
REQUIRE(p.valid());
}
}
for (int i = 0; i < invalid.size(); i++) {
std::stringstream case_title;
case_title << "Testing INVALID: " << invalid[i];
SECTION(case_title.str()) {
REQUIRE(Personnummer::from_string(invalid[i], p));
REQUIRE(!p.valid());
}
}
}
test/catch.hpp
Downloaded from GitHub Repository
General feedback and reviews are very welcome but these are the main questions I had writing this:
- Does the code itself look OK? (It's formatted with
clang-format --style=llvm
) - Are there any issues (big or small) that I've missed (bugs, memory leaks etc).
- How far off C++ pragma is it?
- What would be a more C++ way of things?
- Is the way I split the code in files vs. headers a good/preferred way?
- Are the unit tests written in a good way (structure, location, naming, table driven)?