代碼中的private
成員函數只是一個不必要的複雜因素。我只是將他們的代碼移動到公共成員函數:更少的代碼,更乾淨的代碼,更不直接,因此更直接grokable代碼,都很好。對於其中的一些,您可能可能通過使其在details
名稱空間中成爲自由函數來支持重用,但我認爲這將是過早的泛化,並且可能不會發生可能的重用。
答案結束時的示例代碼。
重新設計的另一個問題,聲明
int minValue();
int maxValue();
排除調用const
對象上這些成員函數。相反,做
int minValue() const;
int maxValue() const;
的第三個問題,這通常是一個非常糟糕的主意™做I/O在非I/O類。如果將樹打印到標準輸出中,您將如何在GUI程序中使用該類?所以,而不是
void printTree();
做例如,
ostream& operator<<(ostream& stream) const;
或例如
string toString() const;
第四個問題,你需要人主複製的 –讀了三個」的「規則和零」的「規則。
做到這一點的最簡單的方法是用
unique_ptr< treeNode<T> > root;
其中unique_ptr
是std::unique_ptr
更換
treeNode<T>* root;
。
或者聲明至少一個複製構造函數和一個複製賦值運算符,或者繼承「不可複製的」類。要使課程有效不可複製,您可以使這些運營商private
或protected
。爲了使它們成爲可複製的,使它們成爲public
並且在每個中都做正確的事情(複製賦值操作符的一個很好的默認實現是通過複製和交換習慣用法來表達它的拷貝構造,這意味着引入一個非拋出swap
函數)。
第五個問題是,實現
template<class T>
int Tree<T>::minValue(treeNode<T>*& root) {
if (root == NULL) { return 0; }
if (root->left == NULL) { return root->data; }
else { minValue(root->left); }
}
強烈建議每個節點存儲的值是隱式轉換爲int
。您不提供treeNode
的聲明。但是,這看起來像是一個設計級別的錯誤,意圖是minValue
返回T
,而不是int
–和同上maxValue
。
一個非常小的編碼問題(沒有設計級別):在C++ 11以後,你應該優先使用nullptr
,不NULL
。
最後,關於
if (root == NULL) { return 0; }
的minValue
,這當然可以被意圖,設計。但是,可能您希望發信號失敗或將該呼叫視爲邏輯錯誤。
將該呼叫視爲錯誤,assert(root != nullptr);
並提供客戶端代碼檢查空樹的方法。
要信號故障,或者與任選的值(例如像boost::optional
或巴頓/ Nackmann的原始Fallible
),或拋出異常(在std::runtime_error
類是一個良好的一般缺省異常類選擇)返回對象。
也可以將兩種方法結合起來,以提供這兩種方法,或許可以使用諸如minValue
和minValueOrX
之類的名稱。
更一般地說,有時候可以保留一些特殊的值作爲「沒有這樣的」指標。例如。 std::numeric_limits<T>::min()
。但是這會產生脆弱的代碼,因爲這樣的值很容易在數據中自然發生,並且由於客戶端代碼可能很難檢查特殊值。
例,編碼C++ 11:
#include <assert.h>
#include <iostream> // std::cout, std::endl
#include <string> // std::string
namespace my {
using std::string;
template<class T>
class Tree
{
private:
struct Node
{
T value;
Node* p_left;
Node* p_right;
auto to_string() const -> string
{
using std::to_string;
string const left = (p_left == nullptr? "" : p_left->to_string());
string const right = (p_right == nullptr? "" : p_right->to_string());
return "(" + left + " " + to_string(value) + " " + right + ")";
}
~Node() { delete p_left; delete p_right; }
};
Node* root_;
Tree(Tree const& ) = delete;
Tree& operator=(Tree const&) = delete;
public:
auto is_empty() const -> bool { return (root_ == nullptr); }
void insert(T const data)
{
Node** pp = &root_;
while(*pp != nullptr)
{
auto const p = *pp;
pp = (data < p->value? &p->p_left : &p->p_right);
}
*pp = new Node{ data, nullptr, nullptr };
}
auto minValue() const -> T
{
assert(root_ != nullptr);
Node* p = root_;
while(p->p_left != nullptr) { p = p->p_left; }
return p->value;
}
auto maxValue() const -> T
{
assert(root_ != nullptr);
Node* p = root_;
while(p->p_right != nullptr) { p = p->p_right; }
return p->value;
}
auto to_string() const -> string
{
return (root_ == nullptr? "" : root_->to_string());
}
~Tree() { delete root_; }
Tree(): root_(nullptr) {}
Tree(Tree&& other): root_(other.root_) { other.root_ = nullptr; }
};
} // namespace my
auto main() -> int
{
my::Tree<int> tree;
for(int const x : {5, 3, 4, 2, 7, 6, 1, 8})
{
tree.insert(x);
}
using std::cout; using std::endl;
cout << tree.to_string() << endl;
cout << "min = " << tree.minValue() << ", max = " << tree.maxValue() << endl;
}
輸出:
((((1) 2) 3 (4)) 5 ((6) 7 (8)))
min = 1, max = 8
@CaptainObvlious因此,這是去了解它的正確方法?我知道這不是一個完整的BST課程,我只是想確保公共/私人界面是正確的方式去做,而且對於一個經驗豐富的程序員來說,這看起來並不是完全荒謬的。 (謝謝!) – New 2014-08-31 00:17:08
樹是一個複雜的結構,沒有一個「單向」來做到這一點。你對正在公開的內容持保守態度,這是一件好事。如果滿足您的需求,您的方式就是正確的。 – 2014-08-31 00:31:01
您存在的問題(需要將對象的引用發送給其自己的成員函數之一)不存在,並且當然不會通過將公共成員函數複製爲私有成員函數來解決。 – 2014-08-31 01:18:15