second by DmitriyRodin · Pull Request #3 · Krestol/CodeBase

DmitriyRodin · 2020-06-09T17:18:04Z

No description provided.

Krestol · 2020-06-10T08:00:56Z

Lesson_9/SmartPointer/Car.cpp

+    std::cout << color_ << " car has been created\n";
+}
+Car::~Car() 
+{ std::cout << color_ << " car has been destroied\n"; }


мелочь. но старайся сразу писать код в одном стиле (см. размещение фигурных скобок здесь и выше)

Krestol · 2020-06-10T08:01:39Z

Lesson_9/SmartPointer/Driver.h

+    Driver(const std::string& name, std::shared_ptr<CarFactory> factory);
+    void BuyCar(const std::string& color);
+    void BuyUsedCar(Driver* d);
+    std::unique_ptr<Car> CellCar();


спасибо. исправил. хорошо что не видела моя учительница по английскому ;-)

Krestol · 2020-06-10T08:02:11Z

Lesson_9/SmartPointer/Driver.h

@@ -0,0 +1,20 @@
+#pragma once
+#include "stdafx.h"


этот здесь не нужен - перенеси в cpp

Krestol · 2020-06-10T08:02:57Z

Lesson_9/SmartPointer/stdafx.h

+#include <iostream>
+#include <string>
+#include <memory>
+#include "Driver.h"


свои файлы лучше по месту подключать

Krestol · 2020-06-10T08:03:28Z

Lesson_9/SmartPointer/Car.h

@@ -0,0 +1,12 @@
+#pragma once
+#include "stdafx.h"


не нужен

Krestol · 2020-06-10T08:03:44Z

Lesson_9/SmartPointer/CarFactory.h

@@ -0,0 +1,9 @@
+#pragma once
+#include "stdafx.h"


не нужен

DmitriyRodin

0

DmitriyRodin · 2020-06-11T07:28:34Z

Lesson_9/SmartPointer/Driver.h

+    Driver(const std::string& name, std::shared_ptr<CarFactory> factory);
+    void BuyCar(const std::string& color);
+    void BuyUsedCar(Driver* d);
+    std::unique_ptr<Car> CellCar();


спасибо. исправил. хорошо что не видела моя учительница по английскому ;-)

DmitriyRodin · 2020-06-11T14:45:44Z

Lesson_9/SmartPointer/autoschool.cpp

+        //std::unique_ptr < Driver> currentDriver(new Driver("Ivan", factory_));
+        //DriversInAutoschool_.push_back(std::move(currentDriver));
+        //secondway: same but less code
+        DriversInAutoschool_.push_back(std::unique_ptr < Driver>(new Driver("Ivan", factory_)));


странно, до разьединения на файлы классов работала вставка драйверов и менеджеров в класс, почему сейчас не работает?! это дело в том что я опять неверно подключил в файлах другие файлы?!

в autoschool.h не видны CarFactory, Driver и DriverManager

DmitriyRodin · 2020-06-12T15:10:13Z

Lesson_9/SmartPointer/DriverManager.cpp

+{
+    std::cout << "go!           ";//it works!
+    //failed to implement through functor and this line stil doesn't work:
+    std::thread th_(&ThreaFunctionManager,OneDriver_);


в этой строке у меня ошибка. подскажи как решить - не собирается даже, чтобы отдебажить (((

реализация через перегрузку оператора крутые скобки тоже не далась мне...

такой вариант тоже не собирается: std::thread th_(&DriverManager::ThreaFunctionManager,&OneDriver_);

Krestol · 2020-06-18T10:49:13Z

Lesson_9/SmartPointer/autoschool.cpp

            DriverManagers_[i].GetOneDriver(std::move(std::unique_ptr<Driver>(DriversInAutoschool_[j].release())));
            j++;
+            DriverManagers_[i].setFiledManagerToOnedDrivers(0);
            DriverManagers_[i].GetOneDriver(std::move(std::unique_ptr<Driver>(DriversInAutoschool_[j].release())));


этот участок точно не дебажил :(
после первого вызова release() в unique_ptr будет nullptr.

будет и ок. дальше j++ и мы передаём драйверов дальше. именно этот участок дебажил вчера. вроде ничего не менял критично

если я не увидел проблемы - прошу укажи мне на неё. сейчас повторю дэбаг этого участка.

тут логика такая что раз я решил, что у менеджера только два водителя - упрощение на этапе создания программы - то вот такой механизм их записи сделал, когда счётчик идёт по номеру менеджера, а драйверов в два раза больше по числу.

на j++
не обратил внимания.

Krestol · 2020-06-18T11:06:04Z

Lesson_9/SmartPointer/Driver.cpp

+        if (!this->HaveCar())
+        {
+            std::vector<int > temp = this->canIbuyUsedCar();
+            int y = temp.empty();


почему int ?

и зачем это здесь вообще?

надо было закоментировать мои строки "для отладки". сори. это только для отладки

Krestol · 2020-06-18T11:07:41Z

Lesson_9/SmartPointer/Driver.cpp

+        bool probaPera = this->HaveCar();
+        if (!this->HaveCar())
+        {
+            std::vector<int > temp = this->canIbuyUsedCar();


судя по имени функции canIBuyUsedCar() она должна возвращать true/false, но ни как ни вектор

да название отстало от реализации. надо переименовать во что-то типа canIBuyUsedCarIFCanGIVEManagerAndDriver()

достаточно буде canIBuyUsedCar

Krestol · 2020-06-18T11:08:28Z

Lesson_9/SmartPointer/Driver.cpp

+        mtx.lock();
+        //std::lock_guard<std::mutex>lock(mtx);
+        this->Go();
+        bool probaPera = this->HaveCar();


именование переменных забавное, но и тебя сбивает и в ревью не понятно

это для дебага - понимать состояние - ещё раз прости. на будущее закоментирую вовсе или оставлю комментарий что это для дебага.

Krestol · 2020-06-18T11:09:10Z

Lesson_9/SmartPointer/Driver.cpp

+    int i = 100;
+    while (i)
+    {
+        mtx.lock();


зачем тут лочится мьютекс?

для того, чтобы покупка и поиск для покупки usedCar прошёл без вмешаетльства других потоков

Krestol · 2020-06-18T11:10:33Z

Lesson_9/SmartPointer/Driver.cpp

 {
-    currentSchool; // ������� ��� ������ �������� � �������
-    return 1;
+    std::vector <int> temp = myManager_->isDriverToBuyCarFromAllManagers_m(mySchool_);


isDriverToBuyCarFromAllManagers_m - судя по названию должна true или false возвращать

возможна и такая логика, но у меня возвращает дополнительно 1 или 2 - и так сразу решает две задачи, чтобы меньше вызовов было, по сути одинаковых.

можешь объяснить, что такое 1, что такое 2 ?

в векторе DriversOwnedByManagers_ у меня жётско зашито - только два драйвера.
1 - значит есть авто у DriversOwnedByManagers_[0] - то есть первого драйвера у этого мэнеджера
1 - значит есть авто у DriversOwnedByManagers_[2] - то есть второго драйвера у этого мэнеджера

если 0 - значит у этого мэнеджера нет драйверов с авто

Krestol · 2020-06-18T11:12:05Z

Lesson_9/SmartPointer/Driver.cpp

+        {
+            std::vector<int > temp = this->canIbuyUsedCar();
+            int y = temp.empty();
+            if (temp.empty())//If temp.empty==1 than meens temp is empty 


empty() возвращает true/false https://en.cppreference.com/w/cpp/container/vector/empty

знаю это и что?!

выше приравниваешь к инту, здесь коммент
//If temp.empty==1

если ты к тому что так (temp.empty()==1) более понятно, то согласен. Буду применять. Если иначе - прошу дать комментарий

я к тому, что "выше приравниваешь к инту, а здесь коммент //If temp.empty==1"
не нужно bool с интом стравнивать.
т.е. в условии if((temp.empty()) правильная запись

Krestol · 2020-06-18T11:14:25Z

Lesson_9/SmartPointer/Driver.cpp

+                std::cout << std::endl << "Get Ready!" << std::endl;
+                //this->BuyUsedCar();
+                //line below doesn't work
+                //car_ = mySchool_->giveUsedCarFromDriversOwnedManagers(temp);//you should work with metods not fields


а можешь тут рассказать какая была идея. что за temp? какая дальше логика в giveUsedCarFromDriversOwnedManagers?

в giveUsedCarFromDriversOwnedManagers() передаётся вектор.

Выше мы смотрим, если он пустой, то в else не заходим. Если не пстой - то в нём записано два числа - первое число это номер мэнеджера в векторе мэнедежров-мэмбер автошколы-DriverManagers_ и второе число - это номер драйвера в векторе DriversOwnedByManagers_ то есть это номер драйвера в векторе - которым владеет только один мэнеджер.

Возвращаемся к giveUsedCarFromDriversOwnedManagers(). Вызывает её водитель, обращаясь к автошколе - своему родителю, передавая ей в виде вектора CoordinatesOfDriverHasCar два числа: номер менеджера и номер драйвера у этого менеджера. Функция giveUsedCarFromDriversOwnedManagers() сама обращается к этому драйверу сразу в return DriverManagers_[CoordinatesOfDriverHasCar[0]] где просит его выполнить метод tellDriverTosellCar, передавая в него номер водителя- он записан во втором элементе вектора CoordinatesOfDriverHasCar[1]. По сути это запрос внутри мэмбера мэнеджера продать машину. При переходу к этому методу видим DriversOwnedByManagers_[index]->SellCar();
И именно вот этот сквозной возврат уникального указателя у меня "не срабатывает". Я понятно изложил заложенную механику?

PS При покупке бу авто я отошёл от логики использования функции BuyUsedCar, потому что драйвер

вообще стало не на много понятнее :)
вектор возвращается из this->canIbuyUsedCar() ? что в ней происходит?

Krestol · 2020-06-18T11:14:48Z

Lesson_9/SmartPointer/DriverManager.cpp

 #include "Car.h"
 #include "autoschool.h"

+int DriverManager::NumberOfManagers = 0;


ага, была попытка статический член использовать, но он не потребовался. С этого улыбаешься или что-то ещё?

статические переменные это уже как мемчик )

Krestol · 2020-06-18T11:16:51Z

Lesson_9/SmartPointer/DriverManager.cpp

    //OneDriver_->Go();
 }
-void DriverManager::ThreaFunctionManager()
+void DriverManager::ThreaFunctionManager()//previous version


у тебя же есть отличный инструмент контроля версий.
подобные комментарии //previous version - тебя же самого запутывают

да, это так. просто сюда я заливаю готовые решение, а где хранить промежуточные - в ходе создания программы и отладки. чтобы быстро "откатиться" назад, оставляю код и комментрию его. Подскажи как делать иначе?! пушить в "свой" репозхиторий?! а потом научиться "из него откатываться" до "предыдущей стабильной версии"?

сюда нужно заливать все промежуточные. почему нет?

Krestol · 2020-06-18T11:17:14Z

Lesson_9/SmartPointer/DriverManager.cpp

+std::vector <int> DriverManager::isDriverToBuyCarFromAllManagers_m(autoschool* currentSchool)
+{
+    std::vector <int> temp = currentSchool->isDriverToBuy_as();
+    bool r=temp.empty();


о, тут bool)

выше уже отписался. вектор тут возвращается. буду иначе именовать функции или разделять на две - одна будет возвращать bool а вторая отрабаывать и возвращать номер,если true

Krestol · 2020-06-18T11:17:48Z

Lesson_9/SmartPointer/DriverManager.cpp

+}
+std::vector <int> DriverManager::isDriverToBuyCarFromAllManagers_m(autoschool* currentSchool)
+{
+    std::vector <int> temp = currentSchool->isDriverToBuy_as();


что означают суффиксы _m _as в именовании функций?

m - функция мэнеджера, а as - функция автошколы

DmitriyRodin · 2020-06-18T15:42:09Z

Lesson_9/SmartPointer/DriverManager.cpp

+{
+       //OneDriver_->Go();
+        DriversOwnedByManagers_[0]->cleverGo();
+        DriversOwnedByManagers_[1]->cleverGo();


почему-то стартует строка 31 в потоке, а строка 32 - нет ((( и того вместо того, чтобы обработать два объекта в одном потоке - и у обоих запустить cleverGo, я запускаю только одного - который вызван в строке 31.

Сам посмотрю ещё. Интересно твоё мнение профессионала - где здесь и что я упустил из виду. Пока не смог найти сам.

Krestol · 2020-06-22T12:51:30Z

Lesson_9/SmartPointer/CarFactory.cpp

+
+std::unique_ptr<Car> CarFactory::BuildCar(const std::string& color)
+{
+    return std::unique_ptr<Car>(new Car(color));


можно make_unique

Krestol · 2020-06-22T12:53:38Z

Lesson_9/SmartPointer/Driver.cpp

+    std::mt19937 gen(rd());
+    std::uniform_int_distribution<> distrib(1, 2);
+    int random_number = distrib(gen);
+    std::lock_guard<std::mutex> locked(mtx);


все драйверы будут выполнять Drive() и операции с покупкой/продажей последовательно, что не ок
нужно сузить область синхронизации до минимально необходимой

Krestol · 2020-06-22T12:54:52Z

Lesson_9/SmartPointer/Driver.cpp

+        }
+    }
+}
+void Driver::Go()//already dont use in prog


если не используется не нужно оставлять

Krestol · 2020-06-22T12:56:49Z

Lesson_9/SmartPointer/Driver.h

+    int intRand(const int& min, const int& max);
+private:
+    std::unique_ptr<Car> car_;
+    std::shared_ptr<CarFactory> factory_;


почему factory в shared, а manager и school в обычном указателе?

потому что они никуда и никому не передаются. в данном контексте, по аналогии с factory - можно было использовать shared_ptr для manager и school. Для практики

а что если удалятся?

я тебя понял. тогда очевидно программа ляжет, так как случится UB. урок данного урока в том что если использовать указатели, то только умные в дальнейшем: shared или unique.

Krestol · 2020-06-22T12:57:25Z

Lesson_9/SmartPointer/DriverManager.cpp

+{
+
+    int i = 4;
+    while (i)


for в таком случае выглядит очевиднее

на будущее принято

Krestol · 2020-06-22T12:58:16Z

Lesson_9/SmartPointer/DriverManager.cpp

+    std::vector<std::thread> threads;
+    for (unsigned currentDriverNumber = 0; currentDriverNumber < DriversOwnedByManagers_.size(); ++currentDriverNumber)
+    {
+        std::random_device rd;


генерация случайного числа дублируется - можно в отдельную функцию вынести

ты имеешь ввиду вынести в отдельную функцию вот эти три строки, под первой из которых ты и оставил этот комментарий?!
std::random_device rd;
std::mt19937 gen(rd());
std::uniform_int_distribution<> distrib(1, 2);

понял. этот код в двух местах, но в этом он не нужен - перенёс его уже полностью в cleverGo();

точнее он там и был. оставил только там

Krestol · 2020-06-22T12:59:51Z

Lesson_9/SmartPointer/DriverManager.cpp

+    {
+        throw std::runtime_error ("...tried to buy a used car but they aren�t\n");
+    }
+    Driver* p_DriverWithCar = PtrDriversWithCar_.back();


что если в элементе контейнера окажется nullptr

плохо будет, но вроде как логикой программы это непредусмотренно. В какой ситуации это возможно?!

Как я понимаю ты предлагаешь тут поставить некую проверку, на всякий случай? Например на случай, если вдруг другой программист поменяет логику функции, которая записывает данные в данный вектор.

Krestol · 2020-06-22T13:00:50Z

Lesson_9/SmartPointer/DriverManager.cpp

+}
+Driver* DriverManager::GivePtrDriverWithCar()
+{
+    if (PtrDriversWithCar_.empty())


обращение к контейнеру синхронизированно между потоками? не получится, что один записывает/удаляет, а другой вызывает empty() ?

сюда мы попадаем из функции cleverGo(). Она и так вся под mutex. При вызове внутри этой функции разве не будет этот mutex распространятся и на вызываемые функции?!

При вызове внутри этой функции разве не будет этот mutex распространятся и на вызываемые функции

будет

Krestol · 2020-06-22T13:02:41Z

Lesson_9/SmartPointer/DriverManager.h

+    std::shared_ptr<CarFactory> factory_;
+    std::vector<std::unique_ptr<Driver>> DriversOwnedByManagers_;
+    static int NumberOfManagers;//TODO �������� ��� � ���� ������
+    std::vector<Driver*> PtrDriversWithCar_;


не возникнет проблем с тем, что driver, который хранится в элементе контейнера будет удален, а в контейнере останется null ?

для этого вроде как pop_back() есть в функции GivePtrDriverWithCar() в предпоследней её строке. Если это не решает проблемы - прошу раскрой где она ещё кроется и натолкни/подскажи решение

см. комментарий выше

Да, это так. Ты прав такое возможно. Во избежание проблемы поставил std::lock_guardstd::mutex locked(mtx); в функцию, которая работает с std::vector<Driver*> PtrDriversWithCar_; (залил в крайний git push)
Как я вижу это решает проблему. Если нет - прошу указать.

вопрос: а можно здесь использовать вектор уникальных указателей?! mutex при этом в любом случае нужно ставить в функции, которые работают с этим вектором, но в целом это работу с вектором ещё больше обезопасит.

вектор умных указателей использовать можно

Krestol · 2020-06-22T13:03:53Z

Lesson_9/SmartPointer/Car.cpp

@@ -0,0 +1,15 @@
+#pragma once


pragma once в cpp не нужно

Krestol · 2020-06-22T13:05:28Z

Lesson_9/SmartPointer/Driver.cpp

+#include "CarFactory.h"
+#include "autoschool.h"
+
+std::mutex mtx;


доступ к какому ресурсу синхронизирует этот мьютекс?

это уже разобрали в переписке в телеграмме. доступ к std::vector<Driver*> PtrDriversWithCar_ - это мембер класса DriverManager

меняю код

Krestol · 2020-06-22T13:06:07Z

Lesson_9/SmartPointer/Driver.cpp

+                {
+                    Driver* p_driverWithCar = myManager_->GivePtrDriverWithCar();
+                    this->BuyUsedCar(p_driverWithCar);
+                    //this->BuyUsedCar(myManager_->GivePtrDriverWithCar()); //same as the two lines above


не оставляй ненужных комментариев

ок. хотел у тебя спросить - строки 50 и 51 в сумме идентичны строке 52?!

да, myManager_->GivePtrDriverWithCar() можно передать прямо в BuyUsedCar()

Krestol · 2020-06-22T13:07:24Z

Lesson_9/SmartPointer/Driver.cpp

+}
+std::unique_ptr<Car> Driver::SellCar()
+{
+    return std::unique_ptr<Car> (car_.release());


машину продал, а у менеджера числится как тот, кто все еще может продать?

не числится, так как есть:
void Driver::BuyUsedCar(Driver* d)
{
car_ = d->SellCar();
std::cout << "I bought a used car\n";
myManager_->GetPtrDriverWithCar(this);
}

просто под покупку бу авто используют отдельную функцию BuyUsedCar. предполагаю что SellCar использовать будут только "свободные" от менеджера Driver'ы.

или есть в чём-то тут проблема?

не совсем очевидно, ну ладно

на будущее учту

DmitriyRodin added 4 commits June 9, 2020 19:44

lesson 09-06-2020 - first push

5ac05ee

chande header

5d8d008

third vertion

1d458b7

autochool in progress

a9c9de8

Krestol reviewed Jun 10, 2020

View reviewed changes

Lesson_9/SmartPointer/Car.h Outdated

@@ -0,0 +1,12 @@

#pragma once

#include "stdafx.h"

Copy link

Owner

Krestol Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не нужен

Krestol reviewed Jun 10, 2020

View reviewed changes

Lesson_9/SmartPointer/CarFactory.h Outdated

@@ -0,0 +1,9 @@

#pragma once

#include "stdafx.h"

Copy link

Owner

Krestol Jun 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

не нужен

DmitriyRodin commented Jun 11, 2020

View reviewed changes

11-06-2020 driverschool and manager with mistake

1ac6608

DmitriyRodin commented Jun 11, 2020

View reviewed changes

12-06-2020

0a8af38

DmitriyRodin commented Jun 12, 2020

View reviewed changes

DmitriyRodin added 3 commits June 13, 2020 09:09

13-06-2020 one driver go no buy use car

b824a87

some mistake 16-06-2020

e82411b

18-06-2020 doesn't work. Please tell me where mystake is

a1a9415

Krestol reviewed Jun 18, 2020

View reviewed changes

DmitriyRodin commented Jun 18, 2020

View reviewed changes

DriverSchool - done. 21-06-2020

7e27bd9

Krestol reviewed Jun 22, 2020

View reviewed changes

22-06-2020 after Denis comments

3270dd2

Conversation

DmitriyRodin commented Jun 9, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DmitriyRodin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!