trainings/CreatingReliableSoftwareCpp/Presentation/good_practise_intro.md

# Good practise
___

## What can we do better?

```C++
class Bar {
public:
    void doSth() {
        std::cout << "Do sth\n";
    }
};

class Foo {
public:
    Foo(std::string name) {
        name_ = name;
        bar_ = new Bar();
    }

    ~Foo() {
        delete bar_;
        bar_ = nullptr;
    }

    void printName() {
        std::cout << name_ << std::endl;
    }

    void doSth() {
        bar_->doSth();
    }

private:
    Bar* bar_;
    std::string name_;
};
```
<!-- .element: class="fragment fade-in" -->
<!-- .slide: style="font-size: 0.62em" -->
___

## Refactor

```C++
class Bar {
public:
    // doSth should be cont method because only print sth
    void doSth() {
        std::cout << "Do sth\n";
    }
};

class Foo {
public:
    // Get string by const& or use std::move when initialize
    Foo(std::string name) {
        // Use initialization list, instead assign inside C'tor
        name_ = name;
        // Avoid usage of new, use unique_ptr and make_unique method
        // Class Foo will be hard to test because we can't substitute bar objects!
        // Bar should be provided in C'tor -> Dependency injection
        bar_ = new Bar();
    }

    ~Foo() {
        // Unneccessary, because we should use unique_ptr
        delete bar_;
        bar_ = nullptr;
    }

    // Should be const method
    void printName() {
        // better use '\n' then endline
        // If you need to flush stream use flush() method
        std::cout << name_ << std::endl;
    }

    // If there is a risk, that ptr may by empty, we shoudl validate it
    void doSth() {
        bar_->doSth();
    }

private:
    Bar* bar_;
    std::string name_;
};
```
<!-- .element: class="fragment fade-in" -->
<!-- .slide: style="font-size: 0.69em" -->
___

## What can we do better?

```C++
class Screen {
public:
    Screen(int height, int width) : height_(height), width_(width) {}
    Screen(int size) : height_(size), width_(size) {}
    ~Screen() {}

    void print(const std::vector<int>& numbers) {
        int current = 0;
        for (int j = 0; j < width_; ++j) {
            std::cout << "_";
        }
        std::cout << '\n';

        for (int i = 0; i < height_; ++i) {
            std::cout << "|";
            for (int j = 0; j < width_; ++j) {
                int num_width = std::to_string(numbers[current]).size();
                std::cout << numbers[current];
                j += num_width;
                if (j < width_) {
                    std::cout << ' ';
                }
                ++current;
            }
            std::cout << "|\n";
        }

        for (int j = 0; j < width_; ++j) {
            std::cout << "_";
        }
        std::cout << '\n';
    }
private:
    Screen() {}
    int height_;
    int width_;
};
```
<!-- .element: class="fragment fade-in" -->
<!-- .slide: style="font-size: 0.62em" -->
___

## Refactor

```C++
class Screen {
public:
    // Use alias -> using Height = int 
    Screen(int height, int width)
        : height_(height), width_(width) {}
    // User probably not expect square screen when initialize with one value
    // C'tor with on argument should be mark as explicit.
    Screen(int size)
        : height_(size), width_(size) {}
    // Not needed here, we also break rule of 5
    ~Screen() {}

    // Should be const method
    void print(const std::vector<int>& numbers) {
        int current = 0;

        // This function repeat twice.
        // Should be separate function like print underscore
        for (int j = 0; j < width_; ++j) {
            // This is not efficient better use std::cout << string(width_, '_') << '\n';
            std::cout << "_";
        }
        std::cout << '\n';

        // This is hard to understand. There is already implemented stream mainpulators
        // like <iomanip> which allow to use `setw` to describe the width ov vlaue
        // or added in c++20 std::format()
        for (int i = 0; i < height_; ++i) {
            std::cout << "|";
            for (int j = 0; j < width_; ++j) {
                // should be const
                int num_width = std::to_string(numbers[current]).size();
                std::cout << numbers[current];
                // do sth with `j` which should be handled by for loop
                j += num_width;
                if (j < width_) {
                    std::cout << ' ';
                }
                ++current;
            }
            std::cout << "|\n";
        }

        // DRY - do not repeat yourself
        for (int j = 0; j < width_; ++j) {
            std::cout << "_";
        }
        std::cout << '\n';
    }

private:
    // By default when we create at least one C'tor, the compiler will not add a default one
    Screen() {}

    int height_;
    int width_;
};
```
<!-- .element: class="fragment fade-in" -->
<!-- .slide: style="font-size: 0.69em" -->