Go to Qiita Advent Calendar 2024 Top
LoginSignup
146
144

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

記事投稿キャンペーン 「2024年!初アウトプットをしよう」
@At-sushi

[海外動画紹介] あなたが止めるべき31の初心者っぽいC++の習慣

Last updated at Posted at 2024-01-31

31 nooby C++ habits you need to ditch - YouTube

この動画の解説が非常に参考になったので、勉強がてら紹介記事を書くことにしました。

完全翻訳ではなく、大雑把に要点のみ解説しています。気になる方は元動画をご参照ください。
登場するソースコードは下記のURLで入手可能です（解説コメント付き）。

翻訳内容に若干自信がない部分があるので、おかしな点があればご指摘いただけると助かります。

1. using namespace std; 1 

#include ...

using namespace std;

void using_namespace_std() {
    string s{"hello, world!"};
    cout << s << endl;
}

関数などのスコープ内に収まっている場合は問題ないが、グローバル空間やヘッダーにusing宣言が定義されている場合、名前空間の衝突の原因になるので、このような使い方は避けた方が良い。2
また、できればオブジェクト単位で個別に指定することを検討した方が良い。

修正例
#include ...

using std::string, std::cout, std::endl;

void using_namespace_std() {
    string s{"hello, world!"};
    cout << s << endl;
}

2. （ループの中などでの）std::endlの複数回使用

void print_range(int start, int end) {
    for (auto i = start; i != end; ++i)
        std::cout << i << std::endl;
}

std::endlは実行毎に出力バッファの内容を逐一flushするので、実行時オーバーヘッドを減らすため、ループの途中などflushの必要がない箇所では改行文字を使用した方が良い。

修正例
void print_range(int start, int end) {
    for (auto i = start; i != end; ++i)
        std::cout << i << '\n';
}

3. range-based forを使用すべき箇所でのindexループの使用

void train_model(const std::vector<int> &data, auto &model) {
    for (std::size_t i = 0; i < data.size(); ++i) {
        model.update(data[i]);
    }
}

typoやバグを避けるため、ループカウンタを参照する必要がないのであればrange-based forを使用した方が良い。

修正例
void train_model(const std::vector<int> &data, auto &model) {
    for (const auto &x: data)
        model.update(x);
}

index変数を使う必要がなくなり、typoやバグを仕込む可能性が少なくなる。

4. STLアルゴリズムの再発明

void know_your_algorithms() {
    const std::vector<int> data = {-1, -3, -5, 8, 15, -1};

    // find the index of the first positive element
    std::size_t first_pos_idx;
    for (std::size_t i = 0; i < data.size(); ++i) {
        if (data[i] > 0) {
            first_pos_idx = i;
            break;
        }
    }
    // use first_pos_idx
}

実行しようとしている処理が単純な場合、すでに用意されているSTLアルゴリズムで代用できないか確かめた方が良い。
上記の例（先頭に最も近い正の要素のindexを取得）の場合、std::find_if関数が使用可能。

修正例
void know_your_algorithms() {
    const auto is_positive = [](const auto &x) {
        return x > 0;
    };
    auto first_pos_it = std::find_if(
            data.cbegin(),
            data.cend(),
            is_positive);
    // or ranges even better
    // use first_pos_it
}

5. Cスタイルの配列の使用

void f(int *arr, int n) {
    // ...
}

Cスタイルの配列を引数として渡した場合、先頭要素を示すポインタ型にキャストされるため、要素数を別途指定する必要がある。
代わりにstd::arrayを使用した方が良い。

修正例
template<std::size_t size>
void better_f(std::array<int, size> &arr) {
    // ...
}

イテレータや、C++20であればstd::spanを使えば範囲を自由に指定できるようになる。

6. reinterpret_castの使用

template<typename T>
void print_bytes(const T &input, std::ostream &os = std::cout) {
    auto *bytes = reinterpret_cast<const std::byte *>(&input);
    os << "[";
    os << std::hex << std::noshowbase;
    for (std::size_t i = 0; i < sizeof(T); ++i) {
        if (i != 0)
            os << " ";
        os << std::setfill('0') << std::setw(2);
        os << static_cast<int>(bytes[i]);
    }
    os << "]\n";
}

C++において、reinterpret_castした変数を安全に参照できるのは元の型に再変換した場合のみであり、それ以外のほとんどのケースでの参照結果は未保証である。3
C++20でビット列を維持したまま型変換するstd::bit_castが実装されたので、変数のビット表現を得たい場合はそちらを使用した方が良い。

修正例
template<typename T>
void print_bytes_cpp20(const T &input, std::ostream &os = std::cout) {
    using bytearray = std::array<std::byte, sizeof(T)>;
    const auto &bytes = std::bit_cast<bytearray, T>(input);
    os << "[";
    os << std::hex << std::noshowbase;
    for (std::size_t i = 0; i < sizeof(T); ++i) {
        if (i != 0)
            os << " ";
        os << std::setfill('0') << std::setw(2);
        os << static_cast<int>(bytes[i]);
    }
    os << "]\n";
}

下記の様なケースにおいて、Cスタイルのキャストはreinterpret_castとして再解釈される可能性があるため、非推奨。

void any_use_of_reinterpret_cast() {
    float y = .123f;
    long i = *(long *) &y;
    // ...
    y = *(float *) &i;
}

7. const_castによるconst属性の除去

const std::string &
more_frequent(const std::unordered_map<std::string, int> &word_counts,
              const std::string &word1,
              const std::string &word2) {
    auto &counts = const_cast<std::unordered_map<std::string, int> &>(word_counts);
    return counts[word1] > counts[word2] ? word1 : word2;
}

const std::map系のコンテナで[]演算子が使えない場合、const属性をキャストで除去するのではなくatメソッドを使用した方が良い。
[]演算子がconst参照でない理由については次項を参照。

8. std::unordered_mapの[]演算子の挙動に対する誤解

std::unordered_map<std::string, int>
count_words(const std::vector<std::string> words) {
    std::unordered_map<std::string, int> counts;
    for (const auto &word: words)
        counts[word]++;
    return counts;
}

std::map系のコンテナに対して[]演算子を用いて存在しないキーを参照した場合、valueをデフォルト値で初期化した上で要素を新規作成するため、[]演算子はconst属性ではない。

9. const属性の未使用

void print_vec_one_per_line(std::vector<int> &arr) {
    for (const auto &x: arr) {
        std::cout << x << '\n';
    }
}

関数が参照先を変更しないことを明示するため、変更しないことが保証されている変数や引数にはconst属性を追加した方が良い。

10. 文字列リテラルのライフタイムに対する誤解

const char *string_literal_lifetimes() {
    return "string literals";
}

文字列リテラルの値はプロセス終了時まで保持されるため、上記の関数は問題なく動作する。

11. 構造化束縛の未使用

void loop_map_items() {
    std::unordered_map<std::string, std::string> colors = {
            {"RED",   "#FF0000"},
            {"GREEN", "#00FF00"},
            {"BLUE",  "#0000FF"}
    };

    for (const auto &pair: colors) {
        std::cout << "name: " << pair.first << ", hex: " << pair.second << '\n';
    }

C++17以降、構造化束縛を使用することにより、可読性を維持しつつメンバ変数に対する参照を複数まとめて初期化することができる。

修正例
    for (const auto&[name, hex]: colors) {
        std::cout << "name: " << name << ", hex: " << hex << '\n';
    }

12. 出力用の参照渡しの使用

void get_values_out_params(const int n, int &out1, int &out2) {
    // do stuff
    out1 = n;
    out2 = n + 1;
}

関数から複数の戻り値を返したい場合、参照渡しではなく専用の構造体を定義して戻り値として返した方が良い。

修正例
struct Values {
    int x, y;
};

Values get_values_return_struct(const int n) {
    return {n, n + 1};
}

戻り値の構造体は自動的にmoveされるので、実行時オーバーヘッドは発生しない。

13. コンパイル時処理できる演算に対する実行時処理4 

constexpr int sum_of_1_to_n(const int n) {
    return n * (n + 1) / 2;
}

void uses_sum() {
    const int limit = 1000;
    auto triangle_n = sum_of_1_to_n(limit);
    // use triangle_n...
}

constexpr属性を指定することで、関数（または変数）を定数式の内部で使用できるようになる。
実行時間短縮の他、テンプレート引数などの定数式が必要な箇所で関数を使用できるようになる。

14. virtual属性が指定されていないデストラクタの使用

class BaseWithNonvirtualDestructor {
public:
    void foo() {
        std::cout << "do foo\n";
    }

    virtual ~BaseWithNonvirtualDestructor() {
        std::cout << "called base destructor\n";
    }
};

class Derived : public BaseWithNonvirtualDestructor {
public:
    ~Derived() override {
        std::cout << "called derived destructor\n";
    }
};

void consume_base(std::unique_ptr<BaseWithNonvirtualDestructor> p) {
    p->foo();
    // deletes p when done
}

void base_with_nonvirtual_destructor() {
    auto derived = std::make_unique<Derived>();
    consume_base(std::move(derived));
}

基底クラス型にキャストしたポインタからクラスを破棄しようとした場合、基底クラスのデストラクタにvirtual属性がないと派生クラスのデストラクタが呼び出されなくなってしまい、クラスが正常に破棄されない可能性がある。

15. メンバ変数の初期化順に対する誤解

template<typename T>
class View {
public:
    View(T *start, std::size_t size) : m_start{start}, m_end{m_start + size} {
        std::cout << "view constructor: " << m_start << "," << m_end << '\n';
    }

private:
    T *m_start;
    T *m_end;
};

void class_member_initialization_order() {
    View<std::byte> v(nullptr, 1);
}

クラスのメンバ変数はコンストラクタ引数の順序ではなく、メンバの定義順に初期化される。

16. 未初期化の変数に対する参照

void default_vs_value_initialization() {
    int x;
    int *x2 = new int;

    struct S {
        int n, m;
        std::string s;

        S() {
            std::cout << "constructor\n";
        }
    };

    S my_s;
    std::cout << my_s.n << " " << my_s.m << " \"" << my_s.s << "\"\n";

C、およびC++の変数はデフォルト初期化を実行した場合不定値を示すケースがあるので、初期値が必要な場合は明示的に初期化する必要がある。

原則として、変数は初期値を指定して定義した方が良い。

修正例
    int y{};
    int *y2 = new int{};
    int *y3 = new int();

    S my_s2{};
    std::cout << my_s2.n << " " << my_s2.m << " \"" << my_s2.s << "\"\n";
}

17. マジックナンバーの過剰使用

float energy(float m) {
    return m * 299792458.0 * 299792458.0;
}

参照先が定数の場合、コンパイラは自動的に最適化を行うので、数値リテラルはなるべく直接使用せずにconstexpr変数などに格納した方が良い。

18. コンテナに対するイテレーションループ内での要素の追加・削除

void modify_while_iterating() {
    std::vector<int> v{1, 2, 3, 4};

    for (auto x : v) {
        v.push_back(x);
    }


    for (auto it = v.begin(), end = v.end() ; it != end; ++it) {
        v.push_back(*it);
    }

    std::copy(v.begin(), v.end(), std::back_inserter(v));


    for (auto x: v) {
        std::cout << x;
    }
    std::cout << '\n';
}

イテレーションの最中にコンテナの要素を追加・削除するとイテレータの指示先が変化する可能性があるため、注意を払う必要がある。

19. 関数の戻り値に対するstd::moveの使用

std::vector<int> make_vector(const int n) {
    std::vector<int> v{1, 2, 3, 4, 5};

    std::vector<int> w{1, 2};

    if (n == 0)
        return std::move(v);
    else
        return std::move(w);
}

関数の戻り値は自動的に右辺値を返すように最適化される。
手動で指定した場合、場合によっては最適化が抑制されてしまうので、避けたほうが良い。

20. std::moveの挙動に対する誤解

以下はstd::moveの実装例である。

template<typename T>
constexpr std::remove_reference_t<T> &&
move(T &&value) noexcept {
    return static_cast<std::remove_reference_t<T> &&>(value);
}

原則としてstd::moveは変数に対する参照を右辺値型にstatic_castするだけであり、メモリ間の移動は一切行わない。

21. 引数の評価順に依存した関数呼び出し

void g(int a, int b, int c) {
    std::cout << a << ' ' << b << ' ' << c << '\n';
}

void function_evaluation_order_not_guaranteed() {
    std::string s = "but i have heard it works even if you don't believe in it";

    int i = 0;
    const auto inc_i = [&i]() -> int {
        return ++i;
    };

    g(inc_i(), inc_i(), inc_i());
}

C++において引数の評価順は規定されておらず、評価順によって結果が変わるコードの動作は未定義である。

22. 不必要なヒープアロケーションの実行

struct Record {
    int id;
    std::string name;
};

void unnecessary_heap_allocations() {
    Record *customer = new Record{0, "James"};
    Record *other = new Record{1, "Someone"};

    // do work

    delete customer;
    delete other;
}

上記の例の場合、通常のスタックアロケーションで問題なく動作する。
newによる初期化を使用する前に、ヒープアロケーションが本当に必要かどうかもう一度確認した方が良い。

23. スマートポインタの未使用

struct Metadata {
    std::byte magic;
    std::size_t size;
    std::array<char, 20> name;
    std::array<char, 200> description;
    // other fields
};

Metadata *read_metadata(const std::ifstream &file) {
    auto *data = new Metadata{};

    // read fields from file

    return data;
}

変数をnewを使用してアロケートした場合、例外処理などによりdelete処理がスキップされた場合にメモリリークが発生してしまう。
あらかじめstd::unique_ptrなどのスマートポインタにアドレス値を格納しておくことで、オブジェクトの消滅時にメモリが自動解放される。

24. 生成用ヘルパー関数を使用しないスマートポインタの初期化

void f(std::unique_ptr<int> a, std::unique_ptr<float> b) {

}

void constructing_a_unique_ptr_directly() {
    f(std::unique_ptr<int>(new int{}),
      std::unique_ptr<float>(new float{}));

std::make_unique及びstd::make_shared関数を使用することにより、コンストラクタ引数をforwardしつつ、オブジェクトの動的確保を自動化することができる。

修正例
    f(std::make_unique<int>(),
      std::make_unique<float>());

25. コンテナ型定義以外でのnew delete構文の使用5 

class Widget {
public:
    Widget() : meta{new Metadata{}} {
        // whatever
    }

    virtual ~Widget() {
        delete meta;
    }

private:
    Metadata *meta;
};

メモリ管理に問題がない使用法であっても、クラス側で独自に管理するのは単一責任の原則から見てふさわしくないので、できるだけスマートポインタを使用したほうが望ましい。

修正例
class BetterWidget {
public:
    BetterWidget() : meta{std::make_unique<Metadata>()} {
        // whatever
    }

private:
    std::unique_ptr<Metadata> meta;
};

26. 手動リソース管理

void read_from_a_file(char *name) {
    FILE *fp = fopen(name, "r");
    // ... work with file, EXCEPTION?
    fclose(fp);

new及びdeleteと同様、例外処理などによりリソースの解放漏れが発生するおそれがあるので、自動的にリソース管理できるオブジェクトがあればそちらを使用した方が良い。

修正例
    std::ifstream input{name};
    // work with the file

このようなオブジェクトによる自動リソース管理をRAIIと呼び、例外安全なコードを書く為の推奨パターンとなっている。

ちなみに、std::unique_ptr及びstd::shared_ptrにテンプレート引数として関数を渡すことで、カスタムデリータを指定することができる。

27. スマートポインタの不必要な使用

std::shared_ptr<int> max(std::shared_ptr<int> a, std::shared_ptr<int> b) {
    return *a > *b ? std::move(a) : std::move(b);
}

std::shared_ptrの処理には実行時オーバーヘッドがあるため、引数などにおいて所有関係を管理する必要がない場合6はstd::shared_ptrを使用せず生ポインタを使用した方が良い。

修正例
const int *max(const int *a, const int *b) {
    return *a > *b ? a : b;
}

28. std::shared_ptrの使い過ぎ

struct Pizza {
    Pizza(float diameter, std::vector<std::string> toppings) :
            m_diameter{diameter}, m_toppings{std::move(toppings)} {}

    float m_diameter; // inches
    std::vector<std::string> m_toppings;
};

std::shared_ptr<Pizza> make_shared_pepperoni_pizza(float diameter) {
    std::vector<std::string> toppings = {"red sauce", "cheese", "pepperoni"};
    return std::make_shared<Pizza>(diameter, std::move(toppings));
}

std::unique_ptr<Pizza> make_unique_pepperoni_pizza(float diameter) {
    std::vector<std::string> toppings = {"red sauce", "cheese", "pepperoni"};
    return std::make_unique<Pizza>(diameter, std::move(toppings));
}

void convert_unique_to_shared_is_easy_and_cheap() {
    auto pizza = make_unique_pepperoni_pizza(16.0f);

    std::shared_ptr<Pizza> shared_pizza = std::move(pizza);
    std::shared_ptr<Pizza> shared_pizza2 = make_unique_pepperoni_pizza(16.0f);
}

前項と同様、参照カウンタが必要ない場合はstd::unique_ptrを使用できないか検討した方が良い。
一時的に引数として利用するだけなら実体を参照渡しするかgetメソッドを使用すればよく、必要であればほぼノーコストでstd::shared_ptrに変換できる。

29. スレッド安全性が要求される場面での不適切なstd::shared_ptrの使用

struct Noisy {
    int x;

    Noisy() : x{0} {

    }

    ~Noisy() {
        std::osyncstream{std::cout} << "Deleted from thread " << std::this_thread::get_id() << '\n';
    }
};


void worker(std::shared_ptr<Noisy> noisy) {
    for (int i = 0; i < 5; ++i) {
        for (int j = 0; j < 10000; ++j) {
            noisy->x++;
        }
        std::osyncstream{std::cout} << "Thread " << std::this_thread::get_id() << " executing: " << noisy->x << "\n";
    }
}


void shared_ptr_is_NOT_threadsafe() {
    std::osyncstream{std::cout} << "Main thread " << std::this_thread::get_id() << " executing\n";

    auto noisy = std::make_shared<Noisy>();
    std::jthread t2(worker, noisy);
    std::jthread t1(worker, noisy);
    noisy.reset();
}

std::shared_ptrの参照カウンタはスレッドセーフであるものの、アドレスの参照にはスレッド安全性が保証されていないため、スレッド安全性が必要な場合はstd::lock_guardなどで制御する必要がある。

30. const型ポインタとconst変数を指すポインタの混同

void const_pointer_vs_pointer_to_const() {
    int x = 0;
    int y = 0;

    const int *ptr_to_const = &x;
    int const *ptr_to_const2 = &x;
    // *ptr_to_const = 1; // NOT ALLOWED
    ptr_to_const = &y;
 
    int *const const_ptr = &x;
    *const_ptr = 1; // x == 1
    // const_ptr = &y; // NOT ALLOWED
}

const int *及びint const *型はconst int型に対する通常のポインタなので、ポインタそのものに対してはconst属性が適用されない。const型のポインタを定義する場合はint * constのように指定する必要がある。

31. （おまけ）コンパイラの警告無視

コード中に未定義動作が含まれている場合、コンパイラはwarningを生成するので、warningを無視すると重要な問題点を見逃してしまう恐れがある。

Python版もあるよ！

25 nooby Python habits you need to ditch - YouTube

参考文献

  1. 訳注：元動画では構文としてのusing namespace stdとその使用(using)が掛けられています。 ↩

  2. プレゼンテーション用の簡易コードなどでは使われる場合があります。 ↩

  3. 元動画では未定義動作となっていますが、C++20規格書では結果が未保証であるとされています。 ↩

  4. 訳注：この例だと実行時処理として扱われるような気がするのですが… ↩

  5. 23とほとんど同じ内容のような気がするが気にしてはいけない ↩

  6. constポインタであるとは限らない。 ↩

146
144
6

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
146
144

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?