C++のコードレビューまとめ

C++のコードレビューのまとめを行います

派生される可能性があるクラスの場合、デストラクタにvirtualをつけよう

ポリモーフィズムを利用すべく作った基底クラスのデストラクタはvirtualが必要であるためです。基底クラスのデストラクタにvirtualを付けない場合、派生クラスのデストラクタが呼ばれずメモリリークが起きる

ルール厳守なもの

[Guilty Code]

class Test
{
    Test(){}
}

[Correct Code]

class Test
{
    Test(){}
    virtual ~Test() = default;
}

【c++】デストラクタにvirtualを付ける場合、付けない場合。
- https://qiita.com/ashdik/items/3cb3ee76137d176982f7

std::cinとstd::coutへのアクセスは副作用があるため呼び出す場所は限定すること

std::cinとstd::coutは副作用があるため呼び出す場所は限定すること (main関数内を強く推奨)

C：cinバッファが空かどうかをどうやって確認するのですか？
- https://codeday.me/jp/qa/20190301/340492.html

不要な#include文は削除しましょう

使用していない#include文は削除しましょう

コードを意味のあるまとめまりにしよう

単体テストを活用しよう

#include <cassert>
#include <string>
#include "Main.h"

bool isIPAddr(const std::string& inputConsoleLine);

class Test
{
public:
    Test()
    {
        ExecuteTest();
    }
private:    
    static void ExecuteTest()
    {
        // ---- 正常パターン単体テスト ---- //
        // 正規表現ロジック上の下限値・上限値確認
        assert(isIPAddr("0.0.0.0"));
        assert(isIPAddr("255.255.255.255"));
        assert(isIPAddr("9.9.9.9"));
        assert(isIPAddr("10.99.10.99"));
        assert(isIPAddr("100.199.100.199"));
        assert(isIPAddr("200.249.200.249"));
        assert(isIPAddr("250.255.250.255"));
        
        // ---- 異常パターン単体テスト ---- //
        // 前置きの0は失敗判定させる
        assert(!isIPAddr("00.00.00.00"));
        assert(!isIPAddr("000.000.000.000"));
        assert(!isIPAddr("0000.0000.0000.0000"));
        
        // 異常値を与える
        assert(!isIPAddr("."));
        assert(!isIPAddr(","));
        assert(!isIPAddr("2147483647"));
        
        // コロンを挿入する位置を変更する
        assert(!isIPAddr("12.3..4"));
        assert(!isIPAddr("..1.2.3.4"));
        assert(!isIPAddr("1.2.3.4.."));
        
        // 誤った桁数を与える
        assert(!isIPAddr("1"));
        assert(!isIPAddr("1.2"));
        assert(!isIPAddr("1.2.3"));
        assert(!isIPAddr("1.2.3.4.5"));
        
        // 範囲外の値を与える
        assert(!isIPAddr("-1.-1.-1.-1"));
        assert(!isIPAddr("256.256.256.256"));
        assert(!isIPAddr("2147483647.2147483647.2147483647.2147483647"));  // __int32の+側最大値
    }
};

const static Test Test;

intよりint32_tを使おう

型エイリアス宣言を用いて、クラス側でクラスで使用する型を決定することで、ソフトウェアの堅牢性を向上させる

型エイリアスは以前に定義された型を参照する名前です (typedef と同様です)

以下のように型を別名で宣言します。 using TestData = std::array<int, 10>;

【修正前】 [Main.cpp]

#include <iostream>
#include "Test.hpp"

int main(void)
{
    std::array<int, 10> testData{1,2,3,4,5,6,7,8,9,10};
    
    Test test{testData};
    
    for (const auto& i : test.GetMultiplyData(5))
    {
        std::cout << i << std::endl;
    }
}

[Test.hpp]

#include <array>
#include <vector>
#include <algorithm>    // std::transform

class Test
{

private:
    std::array<int,10> m_inputData;

public:
    Test(std::array<int,10> inputData) : m_inputData{inputData}
    {}
    
    // 倍数したデータをvectorで取得する
    std::vector<int> GetMultiplyData(int times)
    {
        std::vector<int> resultData;
        
        std::transform(std::begin(m_inputData), std::end(m_inputData), std::back_inserter(resultData), [times](int data){return data * times;});
        
        return resultData;
    }
};

【修正後】 [Main.cpp]

#include <iostream>
#include "Test.hpp"

int main(void)
{
    Test::TestData testData{1,2,3,4,5,6,7,8,9,10};
    
    Test test{testData};
    
    for (const auto& i : test.GetMultiplyData(5))
    {
        std::cout << i << std::endl;
    }
}