Le_Forgeron <jgr### [at] freefr> wrote:
> * you used a template ? (nop, just a red herring for you! but a single
> uppercase letter is usually preferred to Value_t)

  Preferred by who? A single uppercase letter is only obfuscation.

> * you have the [] checks the upper range (whereas usually it's at()
> which check, and [] do not)

  I'm not creating a C++ standard container. If the class specs say that
an assertion failure will happen on an invalid index, then it's ok.

> * you checked upper range, but not lower one

  Why would I check a lower range when std::size_t is by definition
unsigned? ("index >= 0" is always true.)

> * you should have stated index in [] with const

  Why? It doesn't make any difference from the outside, and the
implementation is trivial.

> * using assert is not the right way to handle the issue.

  It depends on how you spec the class. It's certainly the *easiest* way
of doing it.

> * you did not test in creator the value of size ( 0 ?)

  "new Value_t[0]" is perfectly valid C++ and will not cause malfunction.

> * copy-constructor... you're in trouble with mData

  That's the problem with the class.

> * you did not disable automatic conversion of "number" into Array<>, so
> if you have some different signature for some methods using either
> number or Array<>, you're in for surprise (explicit)

  Maybe.

> * you forget the include needed for size_t (cstring)
> * you forget the declaration of assert() (assert.h)

  I skipped #include lines for brevity. (Besides, std::size_t is officially
declared in <cstddef>.)

-- 
                                                          - Warp

Post a reply to this message