Axter wrote:
Noah Roberts wrote: Axter wrote:
Check out the following link for an example:
http://code.axter.com/dynamic_2d_array.h
STILL trying to pass that off as an example are you???
If you are going to show people how to implement something like that at
least put a little effort into it.
Still passing yourself as an expert in this topic.
If you think you can do better, then do-so, and post it as an example.
I am not posting this as an "example" as I don't agree with the idea.
I am also not posting it as an expert sample. I am posting this to
show some of the things a programmer should do differently than you did
if they insist on going that way and why your code "example" should not
be considered such.
If anyone believes the following code exhibits bad practice in any way
please speak up. I won't promise to agree but do listen and in cases
such as this I believe correctness is more important than my ego.
Note the use of whitespace!!
I did keep my lines to 80 chars but this "newsreader " may still break
the formatting. Does anyone have a suggestion of some place to put
this for better readability?
I don't normally comment so verbosely. In this case explainations and
reasoning are the important part...not necissarily the code itself. As
a coding standard I try to make my code itself do the explaining and
not make use of cryptic style that HAS to be commented.
One question I do have for the experts: In the copy constructor I
originally had new T[colCount_ * rowCount_] but this caused occasional
crashes (that were actually not so occasional). It is my understanding
that rowCount_ and colCount_ are initialized before the call to new T[]
so that should have worked. Using other's variables instead fixed the
problem for at least 20 runs so I am confident it is fixed. The
question is, why did it not work the first time, my failure or
compilers? If mine, where is my understanding flawed?
CODE: =============== =============== ====
// This class reimplements the dynamic_2d_arra y template created
// by David Maisonave and can be viewed at:
//
http://code.axter.com/dynamic_2d_array.h
//
// This version fixes some of the bugs and poor coding practices in the
above
// implementation. I do not recommend doing anything like this, but if
you
// must, and want an example, I can't leave you at the mercy of a coder
that
// comes up with the above and calls himself an expert.
//
// Note: I am also not an expert. There are very likely places where
this could
// be improved. My heart is not into it as I disagree with the very
idea.
#include <cstdlib>
#include <cassert>
// Use typename or class? It doesn't really matter. One could
succesfully
// argue that the primary purpose of this class isn't to contain
classes though.
template< typename T >
class dynamic_2d_arra y
{
public:
dynamic_2d_arra y(size_t rowCount, size_t colCount)
: rowCount_(rowCo unt), colCount_(colCo unt),
storage_(new T[rowCount * colCount])
{
/*
* Changes: more descriptive variable names and a lack for a check
* of size for 0. Allocate any time. The reason for the
* later is simple: the check doesn't help, ignores the user's
* intention, and causes undefined behavior if the class is ever
* used.
*
* The correct way to do what the original code does, making sure
this
* class isn't used with 0 bounds is:
*/
assert(rowCount > 0 && colCount > 0);
/*
* You could throw an exception instead. All depends on if this
is an
* incorrect use of the class or a runtime error. I'm choosing
incorrect
* use of the class. Seems logical and is easier to implement.
*/
}
dynamic_2d_arra y(const dynamic_2d_arra y & other)
: rowCount_(other .rowCount_), colCount_(other .colCount_),
storage_(new T[other.rowCount_ * other.colCount_])
{
// In this case there is no need for the check at all as the
creation
// of a dynamic_2d_arra y with 0 bounds has been disallowed.
// Assert anyway to advertise the precondition...
assert(other.ro wCount_ > 0 && other.colCount_ > 0 && other.storage_
!= 0);
std::copy(other .storage_,
other.storage_ + (rowCount_ * colCount_),
storage_);
}
~dynamic_2d_arr ay() { delete [] storage_; }
// These definitions hide the fact that you are actually returning
// a pointer to insides. Clients are discouraged from depending on
this fact
// as it is not advertized by the interface. You could change this
at any
// time and not affect well behaved clients. Without these
definitions any
// changes would propegate to all callers.
typedef T* row_iterator;
typedef const T* const_row_itera tor;
// Col iterator is much more complex and not in the original
interface.
// Personally I think the interface is not complete without this
definition
// but am not willing to spend the time implementing it. For one,
operator[]
// would then be totally inappropriate and implementing that operator
is the
// purpose of the original class.
row_iterator operator [] (size_t row)
{
return storage_ + (row * colCount_);
}
const_row_itera tor operator [] (size_t row) const
{
return storage_ + (row * colCount_);
}
// That is the extent of the original dynamic_2d_arra y. Those are
all the
// messages that object can respond to. Not very dynamic. Here are
some
// more messages that will let this class at least be useful in that
it will
// do MORE, not LESS, than a vector or primitive array.
// bounds checking implementations of the above:
row_iterator row(size_t row)
{
if (!(row < rowCount))
{
throw std::out_of_ran ge("Row index out of range");
}
return storage_ + (row * colCount_);
}
const_row_itera tor row(size_t row) const
{
if (!(row > rowCount))
{
throw std::out_of_ran ge("Row index out of range");
}
return storage_ + (row * colCount_);
}
// No way to protect the row iterator from invalid col bounds because
of the
// implementation. Changing row_iterator to a smarter object could
fix that.
// provide sizing knowledge, needed by almost any client:
size_t rowCount() const { return rowCount_; }
size_t colCount() const { return colCount_; }
// provide resizing - make the class live up to its name:
void resize(size_t rowCount, size_t colCount)
{
assert(rowCount > 0 && colCount > 0);
T * temp_storage = new T[rowCount * colCount];
memset(temp_sto rage, 0, sizeof(T) * rowCount * colCount);
size_t rows = std::min(rowCou nt, rowCount_);
size_t cols = std::min(colCou nt, colCount_);
for (size_t i = 0; i < rows; ++i)
{
std::copy(stora ge_ + (i * colCount_),
storage_ + (i * colCount_) + cols,
temp_storage + (i * colCount));
}
// operations that could cause an exception are finished.
// None of the following can. Strong guarantee...
std::swap(stora ge_, temp_storage);
rowCount_ = rowCount;
colCount_ = colCount;
delete [] temp_storage;
}
/*
* The lack of this operator is a major deficiency of the original
class.
* At one point it was hidden under the "protected" scope and I
mentioned that
* this was not the best as the class cannot be overriden anyway.
The
* original was better at that time but has since been "improved" to
not have
* or hide this operator. This means it is implemented by default
and will
* generate undefined behavior the first time it is used and the
objects
* destroyed. */
dynamic_2d_arra y & operator = (dynamic_2d_arr ay other)
{
// Strong exception guarantee...
std::swap(stora ge_, other.storage_) ; // If this is new to you look
closely.
std::swap(rowCo unt_, other.rowCount_ );
std::swap(colCo unt_, other.colCount_ );
/* note that 'other' now owns my old storage and will delete it
when this
* function exits. */
}
// To finish the *advertized* interface of the class as commented in
the
// original we must provide direct access to the data storage for C
functons
// that need it. This is reasonable but dangerous.
// Probably better ways to do this:
T * c_array() { return storage_; } // no reason to pretend we are not
doing
// what we are doing...
// Comments should reflect the inherint danger of calling the above
function
// at the least. Use should be highly discouraged. Better yet,
don't
// implement it at all and/or do something better. Perhapse the more
costly
// but more correct way:
// std::auto_ptr<T > c_array() const;
/*
* other useful interface items that could be implemented:
* * explicit constructor from c array.
* * functional interface to get T object as f() or operator ().
*
* Do not be tempted to implement a non-explicit constructor from a c
array
* or to implement an implicit conversion to T* or T**.
*/
private:
T * storage_;
size_t rowCount_;
size_t colCount_;
};
#include <iostream>
int main()
{
typedef dynamic_2d_arra y<int> int_array;
int_array a1(5, 7);
assert(a1.colCo unt() == 7);
assert(a1.rowCo unt() == 5);
a1[3][5] = 20;
a1[4][6] = 30;
assert(a1[3][5] == 20);
assert(a1[4][6] == 30);
a1.resize(7, 10);
assert(a1[3][5] == 20);
assert(a1[4][6] == 30);
a1.resize(4, 6);
assert(a1[3][5] == 20);
int_array a2(a1);
assert(a1[3][5] == 20);
a1[1][1] = 100;
assert(a1[3][5] == 20);
assert(a1[1][1] == 100);
a2 = a1;
assert(a1[3][5] == 20);
assert(a1[1][1] == 100);
assert(a2[3][5] == 20);
assert(a2[1][1] == 100);
std::cout << "ALL IS WELL WITH THE INTERFACE.\n";
return 0;
}
