search

indiantarget / quimeraengine

0 stars 0 forks source link

QFixedArray 002 #467

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
One of the parts of the development of QFixedArray class.

Task description:

https://drive.google.com/file/d/0B0FuyHpDsrcfaFZwbFJscWx2dDA/edit?usp=sharing

Original issue reported on code.google.com by kinesiscontact@gmail.com on 22 Jan 2014 at 12:55

GoogleCodeExporter commented 9 years ago

Original comment by Lince3D@gmail.com on 22 Jan 2014 at 12:57

GoogleCodeExporter commented 9 years ago

Original comment by Lince3D@gmail.com on 22 Jan 2014 at 1:00

GoogleCodeExporter commented 9 years ago

Original comment by Lince3D@gmail.com on 22 Jan 2014 at 1:01

GoogleCodeExporter commented 9 years ago

Original comment by Lince3D@gmail.com on 22 Jan 2014 at 1:08

GoogleCodeExporter commented 9 years ago

Original comment by Lince3D@gmail.com on 2 Apr 2014 at 6:31

GoogleCodeExporter commented 9 years ago

Original comment by Raul.San...@gmail.com on 9 Apr 2014 at 10:00

GoogleCodeExporter commented 9 years ago 
Note: 
Constructor_IteratorPointsToForwardEndPositionWhenUsingInvalidPosition_Test 
fails

Original comment by Raul.San...@gmail.com on 27 Apr 2014 at 9:45

Attachments:

GoogleCodeExporter commented 9 years ago 
Results of the review:

QFixedArray.h
----------------
1-Documentation of GetIterator: Normally, we do not tell the user which 
assertions may fail since they are internal parts and they are not even always 
active.

2-GetIterator, GetFirst and GetLast must be constant.

3-GetIterator:
---Parameters must be constant.
---"if (uIndex >= (this->GetCount() - 1))" --> This is erroneous: 
this->GetCount() - 1 should be just this->GetCount().
---It is recommendable not to write more than one return.
---GetLast returns the last element, not the forward end position.
---It would be enough to write "return QArrayIterator(this, uIndex);".

4-GetFirst and GetLast: 
---The variables firstIndex and lastIndex should be prefixed with u-. However, 
since they are not modified, they could be constant and follow constant's 
naming convention.
---In the documentation, it is not explained what happens when the array is 
empty (QFixedArray cannot be empty but QDynamicArray can be).
---There is a commented line //it.MoveLast();.

5-using Kinesis::QuimeraEngine::Common::DataTypes::u16_q; --> Is this really 
necessary?

QFixedArray_Test.cpp
---------------------
6-Tests should appear in the same order they do in the real code.

7-Missing tests for GetFirst and GetLast: assertions should fail if the array 
is empty.

8-BOOST_CHECK_EQUAL(true, true); ?
//BOOST_CHECK_EQUAL(true, true);

9-Leave a blank line between #if and the documentation of the test.

10-Arrays should be prefixed with ar-.

11-Remove the blank line after // [Verification], in 
GetIterator_AssertionFailsWhenParameterIsBiggerThanArraySize_Test.

12-Leave a blank line before #endif.

13-GetIterator_ReturnsIteratorToLastPositionInTheArrayWhenParameterIsBiggerThanA
rraySize_Test: This test is erroneous, the returned iterator msut point to the 
forward end position, not to the last element.

Original comment by Lince3D@gmail.com on 1 May 2014 at 12:44

GoogleCodeExporter commented 9 years ago

Original comment by Raul.San...@gmail.com on 8 May 2014 at 6:25

Attachments:

GoogleCodeExporter commented 9 years ago 
Results of the review:

1-Documentation of 
GetFirst_ReturnsIteratorToBackwardEndPositionInTheArrayWhenArrayIsEmpty_Test 
and GetLast_ReturnsIteratorToForwardEndPositionInTheArrayWhenArrayIsEmpty_Test:
---"empty array is passed" => "the array is empty".

2-QFixedArrayTestClass fails to compile with GCC. It is necessary to add a 
using:
using QFixedArray<T>::m_pAllocator;

Original comment by Lince3D@gmail.com on 9 May 2014 at 1:45

GoogleCodeExporter commented 9 years ago

Original comment by Raul.San...@gmail.com on 10 May 2014 at 5:29

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by Lince3D@gmail.com on 10 May 2014 at 9:09