2017-08-26 100 views
1

我在其他地方搜索了這裏,找到了解決我的問題和疑慮的方法,但沒有找到任何答案。C++ 11 - 在結構向量中修改結構的成員

限制條件:

  1. 我使用C++ 11一個嵌入式設備上。
  2. 我不能使用std::string
  3. 我不能使用std::make_unique()(但我可以用std::unique_ptrnew
  4. 我不能使用strcpy_s()

問題我有

我遇到的主要問題是在AvailableZones::upsertZone方法中,如果它尚未存在(使用name參數作爲「鍵」),我想向矢量添加一個區域。如果存在,我想更新該區域的溫度和溼度成員。 「添加」部分起作用,但更新部分不起作用。

我的下一個問題是在AvailableZones::findZone成員。我希望能夠返回一個區域,使得調用者不負責釋放/刪除返回的值。

關注:

作爲新的C++,我敢肯定我沒有做很多事情正確的C++ 11的方式。我開放(實際上渴望)任何/所有的指導。 在AvailableZones::findZone方法我想返回Zone我有,而不是創建一個副本或使用new/malloc。看起來我需要使用常規的for/while循環?我已經看到了一些迭代器代碼,但它看起來容易混淆/複雜,但我不確定是否使用迭代器也可以解決這個問題。

最佳實踐相關的問題:

  1. Zone結構的析構函數,如果我用delete,它會導致和 例外,當我運行的代碼。我顯然做錯了什麼。
  2. Zone結構中,我可以使name成員爲std::unique_ptr?如果 那麼,如何?我嘗試了很多方法,但我無法獲得 的編譯或工作。
  3. 是否有更好的方法來實現Zone的構造函數?

守則

我已經把註釋代碼解釋方法的意圖,以及在我需要幫助。

#include "stdafx.h" 
#include <iostream> 
#include <assert.h> 
#include <memory> 
#include <vector> 

using namespace std; 

struct Zone { 
    Zone() {} 
    Zone(const char* name, const float temperature, const float humidity) 
    { 
     auto bufferSize = snprintf(NULL, 0, "%s", name); 
     this->name = new char[bufferSize + 1]; 
     strcpy(this->name, name); 
     this->temperature = temperature; 
     this->humidity = humidity; 
    } 

    ~Zone() { 
     // deleting name here causes an Exception 
     //delete [] name; 
    } 

    char* name = nullptr; 
    float temperature = 0.0f; 
    float humidity = 0.0f; 
}; 

class AvailableZones { 
public: 
    AvailableZones::AvailableZones() { 
     m_zoneVec = std::vector<Zone>(); 
    } 

    ~AvailableZones() { 
    } 

    /* 
     Using Arguments, add a Zone to the private zoneVec member is it does not exist 
     If is does exist (names of zones are unique and used as the "key"), then update 
     the temperature and humidity of the existing zone with those in the arguments 
    */ 
    void AvailableZones::upsertZone(const char *name, const float temperature, const float humidity) { 

     for (auto zone : m_zoneVec) { 
      if (strcmp(zone.name, name) == 0) { 
       zone.temperature = temperature; 
       zone.humidity = humidity; 
       return; 
      } 
     } 

     m_zoneVec.push_back(Zone(name, temperature, humidity)); 
    } 

    /* 
     Given a Zone name, find the zone and return it 
     If a Zone with the given name does not exist 
     return a nullptr 
    */ 
    const Zone *AvailableZones::findZone(const char *name) const { 

     for (auto zone : m_zoneVec) { 
      if (strcmp(zone.name, name) == 0) { 
       // I know this is not correct. 
       // How would I do this, without using "new" and thus 
       // forcing the caller to be responsible for deleting? 
       return &zone; 
      } 
     } 

     return nullptr; 
    } 

private: 
    std::vector<Zone> m_zoneVec; 
}; 


int main() 
{ 
    auto livingRoom = "Living Room"; 
    AvailableZones availableZones; 
    availableZones.upsertZone("Master Bedroom", 72.0f, 50.0f); 
    availableZones.upsertZone(livingRoom, 70.0f, 48.0f); 
    availableZones.upsertZone("Study", 68.0f, 46.0f); 

    auto foundZone = availableZones.findZone(livingRoom); 
    cout << foundZone->name << endl; 
    cout << foundZone->temperature << endl; 
    cout << foundZone->humidity << endl; 

    assert(strcmp(livingRoom, foundZone->name) == 0); 
    assert(70.0f == foundZone->temperature); 
    assert(48.0f == foundZone->humidity); 

    availableZones.upsertZone(livingRoom, 74.0f, 52.0f); 

    foundZone = availableZones.findZone(livingRoom); 

    assert(strcmp(livingRoom, foundZone->name) == 0); 
    assert(74.0f == foundZone->temperature); 
    assert(52.0f == foundZone->humidity); 

    return 0; 
} 

編輯: 下面的代碼實現了由@ max66以及@沃恩卡託和@Artemy維索茨基提出了建議。此代碼現在按照我的要求工作。已作出以下更改:

  1. 基於範圍的循環正在使用引用(或常量引用爲 的情況)。默認基於範圍的循環提供了 值(這也是由@Vaughn Cato提出的)
  2. upsertZone方法中,我使用emplace_back(),以便在容器提供的位置創建Zone實例。與push_back()(較早的代碼)臨時副本正在創建,只是被扔掉(我假設,因爲我沒有一個移動構造函數實現)。
  3. 對strprintf使用strlen(如@ArtemyVysotsky建議),允許我在Zone構造函數中使用初始化列表。
  4. 實現拷貝賦值運算符Zone &operator=(Zone &other)
  5. 實現的拷貝構造函數
  6. 實現移動賦值運算符Zone &operator=(Zone &&other)
  7. 實現移動構造

發現: 每次我的元素添加到載體。先前的元素被「複製」到新的容器位置,並且早期的元素被破壞。我希望他們會被移動而不是被複制。我不確定是否有需要做的事情來確保它們被移動而不是被複制。

Furher更新 看起來,爲了使用Move構造函數,它需要是noexcept。一旦我這樣做,現在沒有任何更改的相同代碼將使用Move而不是Copy。

工作代碼按建議作出

struct Zone { 
    Zone() {} 
    Zone(const char* name, const float zoneTemperature, const float zoneHumidity) 
     :name(strcpy(new char[strlen(name) + 1], name)) 
     ,temperature{ zoneTemperature } 
     ,humidity {zoneHumidity} 
    { 
     cout << "Zone constructor: " << name << endl; 
    } 
    /* Copy Constructor */ 
    Zone(Zone const& other) 
     :name(strcpy(new char[strlen(other.name) + 1], other.name)) 
     ,temperature{ other.temperature } 
     ,humidity{ other.humidity } 
    { 
     std::cout << "In Zone Copy Constructor. name = " << other.name << ". Copying resource." << std::endl; 
    } 
    /* Move Constructor */ 
    Zone(Zone&& other) noexcept 
     : name(nullptr) 
     , temperature(0.0f) 
     , humidity(0.0f) 
    { 
     std::cout << "In Zone Move Constructor. name = " << other.name << ". Moving resource." << std::endl; 

     // Copy the data pointer and its length from the 
     // source object. 
     name = other.name; 
     temperature = other.temperature; 
     humidity = other.humidity; 

     // Release the data pointer from the source object so that 
     // the destructor does not free the memory multiple times. 
     other.name = nullptr; 
     other.temperature = 0.0f; 
     other.humidity = 0.0f; 
    } 

    ~Zone() 
    { 
     cout << "Zone Destructor: " << name << endl; 
     delete[] name; 
    } 

    /* Copy Assignment Operator */ 
    Zone& operator=(Zone const& other) { 
     std::cout << "In Zone Copy Assignment Operator. name = " << other.name << "." << std::endl; 
     Zone tmpZone(other); 
     std::swap(name, tmpZone.name); 
     std::swap(temperature, tmpZone.temperature); 
     std::swap(humidity, tmpZone.humidity); 
     return *this; 
    } 

    /* Move Assignment Operator */ 
    Zone& operator=(Zone&& other) noexcept { 
     std::cout << "In Zone Move Assignment Operator. name = " << other.name << "." << std::endl; 

     if (this != &other) 
     { 
      // Free the existing resource. 
      delete[] name; 

      // Copy the data pointer and its length from the 
      // source object. 
      name = other.name; 
      temperature = other.temperature; 
      humidity = other.humidity; 

      // Release the data pointer from the source object so that 
      // the destructor does not free the memory multiple times. 
      other.name = nullptr; 
      other.temperature = 0.0f; 
      other.humidity = 0.0f; 
     } 

     return *this; 
    } 

    char* name = nullptr; 
    float temperature = 0.0f; 
    float humidity = 0.0f; 
}; 

class AvailableZones { 
public: 
    AvailableZones::AvailableZones() { 
     m_zoneVec = std::vector<Zone>(); 
    } 

    ~AvailableZones() { 
    } 

    /* 
     Using Arguments, add a Zone to the private zoneVec member is it does not exist 
     If is does exist (names of zones are unique and used as the "key"), then update 
     the temperature and humidity of the existing zone with those in the arguments 
    */ 
    void AvailableZones::upsertZone(const char *name, const float temperature, const float humidity) { 

     for (auto &zone : m_zoneVec) { 
      if (strcmp(zone.name, name) == 0) { 
       zone.temperature = temperature; 
       zone.humidity = humidity; 
       return; 
      } 
     }  

     m_zoneVec.emplace_back(name, temperature, humidity); 
    } 

    /* 
     Given a Zone name, find the zone and return it 
     If a Zone with the given name does not exist 
     return a nullptr 
    */ 
    const Zone *AvailableZones::findZone(const char *name) const { 

     for (auto const &zone : m_zoneVec) { 
      if (strcmp(zone.name, name) == 0) { 
       return &zone; 
      } 
     } 

     return nullptr; 
    } 

private: 
    std::vector<Zone> m_zoneVec; 
}; 

void doWork() { 
    static_assert(std::is_nothrow_move_constructible<Zone>::value, "Zone should be noexcept MoveConstructible"); 
    auto livingRoom = "Living Room"; 
    AvailableZones availableZones; 
    availableZones.upsertZone("Master Bedroom", 72.0f, 50.0f); 
    availableZones.upsertZone(livingRoom, 70.0f, 48.0f); 
    availableZones.upsertZone("Study", 68.0f, 46.0f); 

    auto foundZone = availableZones.findZone(livingRoom); 
    cout << foundZone->name << endl; 
    cout << foundZone->temperature << endl; 
    cout << foundZone->humidity << endl; 

    assert(strcmp(livingRoom, foundZone->name) == 0); 
    assert(70.0f == foundZone->temperature); 
    assert(48.0f == foundZone->humidity); 

    availableZones.upsertZone(livingRoom, 74.0f, 52.0f); 

    foundZone = availableZones.findZone(livingRoom); 
    assert(strcmp(livingRoom, foundZone->name) == 0); 
    assert(74.0f == foundZone->temperature); 
    assert(52.0f == foundZone->humidity); 

    foundZone = availableZones.findZone("Non Existent Zone"); 
    assert(foundZone == nullptr); 
} 


int main() 
{ 
    doWork(); 
    return 0; 
} 
+0

如果使用C++ 11,你爲什麼使用'malloc'? – Zereges

+0

我已經經歷了許多次迭代,我想這只是一些剩餘的實驗:)。更新代碼以刪除使用malloc –

+1

將'for(auto zone:m_zoneVec)'更改爲'for(auto&zone:m_zoneVec)' –

回答

1

是沒有用的(並且是危險的)從findZone()返回nullptr,如果你不檢查,如果返回的指針是nullptr與否

auto foundZone = availableZones.findZone(livingRoom); 
cout << foundZone->name << endl; 

有很多不同的方法來解決您的問題findZone();爲了避免問題,我建議避免使用指針並返回元素的副本(但你必須寫一個拷貝構造函數);但是,如果你真的想返回一個指針,你可以重寫的功能如下

Zone const * findZone(const char *name) const { 
    for (auto const & zone : m_zoneVec) { 
     if (strcmp(zone.name, name) == 0) { 
      return & zone; 
     } 
    } 

    return nullptr; 
} 

點是使用const(因爲方法const參考的元素m_zoneVecauto const & zone : m_zoneVec;觀察&)當你現在使用一個臨時複製auto zone : m_zoneVec;沒有&所以複製和不參考)。因此,您可以返回矢量元素的指針,而不是立即銷燬的臨時對象的指針。

您在upsertZone()有完全一樣的問題:你循環測試和(萬一)修改元素的副本矢量

for (auto zone : m_zoneVec) { // DANGER: zone is a **copy** 
     if (strcmp(zone.name, name) == 0) { 
      zone.temperature = temperature; 
      zone.humidity = humidity; 
      return; 
     } 
    } 

所以你修改被立即銷燬複印件;原始Zone是未觸及過的。

您必須修改參考

for (auto & zone : m_zoneVec) { // with & zone is a **reference** 
     if (strcmp(zone.name, name) == 0) { 
      zone.temperature = temperature; 
      zone.humidity = humidity; 
      return; 
     } 
    } 

但是,非常非常重要的,你應該創建一個拷貝構造函數(和,也許,一招構造函數);一個拷貝構造函數,用於爲名稱分配一個新數組(new);否則有複製指針的默認拷貝構造函數。

因此,通過舉例來說,當你寫

m_zoneVec.push_back(Zone(name, temperature, humidity)); 

您創建一個臨時對象,將其推入向量通過創建的副本和臨時的破壞。如果delete,則在啓用Zone析構函數時,銷燬臨時deletename,並且向量中的值使用指向空閒區的name。從這一點來看,程序的行爲是不確定的,無論如何,當availableZone被銷燬時(在程序結束時),delete通過在 - > crash之前被刪除的指針調用!

你能避免使用emplace_back()在插入副本(我建議這個)

m_zoneVec.emplace_back(name, temperature, humidity); 

m_zoneVec加入更多的元素,你可能會導致載體的搬遷,所以移動副本和破壞。

如果您可以使用std::unique_ptr,我想你也可以使用std::shared_ptr

一種可能的替代方案的明確的複製和移動的構造創建是利用插入在智能指針(唯一的或共享的一個name的,根據使用的AvailableZones

+0

將名稱存儲爲智能指針的小評論 - 因爲它被分配了新的[]自定義刪除器,因此將會調用delete [],而不是刪除。 –

+0

@ max66,很多好的建議,解釋和例子!謝謝。我將實施您的建議,並在此處發佈代碼。析構函數中的'delete name'不再拋出異常。但是,我仍然像有內存泄漏一樣下跌(區域名稱)。但是,一旦我完成,讓我發佈我的代碼。 –

+0

@ max66,是多次刪除(由於你在你的回答中解釋)是異常的原因。謝謝。此外,現在我更好地理解發生了什麼(感謝你的好解釋),我之前提出的內存泄漏評論是無效的。 –