Monday, 10 January 2011

ASD / CA1 Assignment Feedback and Comments Part 1 General Feedback

I've finally finished marking all the initial designs for the assignments and I decided to collate some of my thoughts / feedback here to make it easier for you all to see. I've also written individual comments on the assignment sheets (if you can read my handwriting !) so hopefully this will fill in some of you unanswered questions.

Coding Standard
I noticed with a lot of the class diagrams and pseudo code people were not following the coding standard here lots of people we doing things like my_function or position when it should be myFunction and m_position. You get marks for the coding standard as part of the assessment so please have a read of it and try to follow it.

It is important to follow standards as it has two major advantages

1. Integration of your code into the main NCCA code base is a lot easier (and your code may be used by other students next year)
2. You will have to use a coding standard in industry so following one is good practice.
Our coding standard is based on a couple of companies standards so it is quite realistic of things you may encounter in the future.

I also noticed several cases of strange / non-descriptive attribute / method names. Try to make you code read like a narrative as much as possible (and use loads of comments). One that really stuck out was m_vector : ngl::Vector.

So it's a vector but what is the vector representing? Try to be as descriptive as possible, modern editors will use code completion so will save typing and it will make your code easier to maintain and debug.

Use enums
enums are a good way of defining constants and making code "self documenting" I saw one example of code presented which looked like this
if (type == 1)
{
... do something
}
else if (type == 2)
{
... do something else
}

Reading this code is hard to tell what 1 and 2 actually are. More maintainable code would look like this
enum Enemies{Tank,Helicopter};
Enemies enemyType=Tank;

if(enemyType==Tank)
{
... do something
}
else if (enemyType == Helicopter)
{
... do something else
}

The above code defines an enum called Enemies which can now be used as a data type, we can only assign it the values defined in the enum and thus makes the code easier to maintain / manage. By default the first value will be assigned 0. We can also add enums to the public area of a class and use the scope resolution operator to access it (see ngl::Material for good example e.g.  ngl::Material::GOLD)

Arrays
I saw quite a lot of attributes which were m_data : array or m_data[] : float etc etc. Wherever possible do not use an array in your programs, they will be a fixed size and makes your program more prone to overflow errors and you may also need to change the size. Where you need to store data in an array type format use a std::vector these are dynamic arrays which can be set to a fixed size to start with and then we can add or remove other elements when we wish. Performance wise they are almost the same as a normal array and the data is guaranteed to be contiguous so we can use it with other libraries etc.

If a std::vector is not suitable for for you needs then use dynamic allocation of the data using new. For example

float *volumeData = new float[WIDTH*HEIGHT*DEPTH];


Where to Start
The main thing about getting you all to do the design first is it will make the writing of the code easier in the long run. You should all now have class diagrams with a list of attributes and methods. You can now start creating these classes and testing them.

First generate a .h / .cpp file for each of the classes and define the class as follows

#ifndef __MYCLASS_H__
#define __MYCLASS_H__
class MyClass
{
public :

private :

};
#endif


Put all the attributes in the private section, and the methods in the public / private elements where appropriate. Once you have done this prototype each of the methods in the .cpp file or if mutators and accessors they may be placed inline in the .h file.

Next you need to write a test harness to check each of the methods and see if they all work. Obviously in more complex systems other classes may be dependent upon each other so work in a peice-wise fashion to get all the different elements working.

It is best to compile you code often and write small bits test and repeat, this will make it easier to spot error and try to reduce error creep.