Title: Student Code Critique Competition 11

OK let us try something slightly different this time. Go back to my interpretation of the student's task (a class to manage the subjects taken and marks attained by a single student) and write a model answer. All types you need should be properly implemented (note that subjects and marks are types) in an appropriate fashion. Now explain clearly your reasons for the different design decisions you made.

Well, confusion indeed! The original contributor obviously had his idea of what the specification was, the teacher had another idea, and then FG had yet another idea! I've chosen to continue with the original contributors idea of having a class that encapsulated a 'set of students scores', so arguably a better name for it would be StudentRecord or StudentScores. However, I've continued to call the class Student, in order to maintain some continuity with what has gone before.

First, let me restate the problem specification (Francis's version): "Implement a class to record an individual student's marks. The student may be taking up to 10 subjects. Subjects are identified by a subject number. Write a short program to test the class." Note that there is no indication of what context the resulting class will be used in, something I will look at later on as it will affect our implementation.

I will write some code based on the approach taken by the original student (including old-style C++), then at the end I will show how the specification can be implemented using the latest style of C++ coding.

Let me start off with the test bed, often a good starting point, because this clarifies what we require the class to do.

#include "student.h"
int main() {
  Student s1;
  Student s2(1, 32);
  s1.add(2, 22);
  s2.add(0, 0);
  s2.add(1, 56);
  s2.add(2, 22);
  s1.output();
  s2.output();
}

Here we create 2 students, one using the default constructor, the second using the parameter constructor. Then we add some values and then ask the objects to print themselves out. I have simplified a bit here, as there would be some further tests that could be run.

Here is a possible implementation of the Student class:

In file student.h

#include <iostream.h>
const int MAX_ENTRIES = 10;
typedef int subjcode_t;
typedef int score_t;
class Cstudent {
public:
  CStudent():m_numGrades(0){}
  CStudent( subjcode_t g, score_t s );
  int add( subjcode_t g, score_t s );
  void output( ostream& os );
private:
  subjcode_t m_subj[MAX_ENTRIES];
  score_t m_score[MAX_ENTRIES];
  int m_numGrades;
};

In file student.cpp

#include "student.h"
CStudent::CStudent(subjcode_t g, score_t s):m_numGrades(0) {
  add(g, s);
}
int CStudent::add(subjcode_t g, score_t s){
  if( m_numGrades > MAX_ENTRIES ) {
    return MAX_ENTRIES;
  }
  m_subj[m_numGrades] = g;
  m_score[m_numGrades] = s;
  return m_numGrades++;
}
void CStudent::output(std::ostream& os){
  os<< "Student: number entries: = " << m_numGrades << std::endl;
  for( int i = 0; i < m_numGrades; i++ ) {
    os << "Grade: " << m_subj[i] << " Score: " << m_score[i] << std::endl;
  }
}

So, a few comments on the above.

I've used a typedef for the subject code and for the grade. This is not really necessary, but could be useful in the future if we needed to change the type of either (to an unsigned int, for example).

[OK, now if I were marking this as student work I would mark you down because the question as set specifically required that these would be types. I would expect, at a minimum:

struct subject {
  int code;
};

FG]

The add(..) method returns an int. We have the choice of returning a void, a bool or an int. Using a void return would be a waste of a return value, we could return a bool to indicate if the values were inserted correctly. But returning the index of the inserted value gives the caller an indication of how many grades have been inserted so far. I have also simply ignored any insertions past the maximum number. This may not be the best response to an out of bounds insertion, see later for more on this.

The output(...) method takes an ostream parameter, this allows us to use the same method and redirect the output wherever we want, to a file or into a strstream. (or even a standard stringstream FG)

Using two arrays (m_subj and m_score) is probably unnecessary. It is quite likely that given there is a fixed number of entries (10 in this case) that there is fixed number of courses, and that we would only need to keep track of the score for each course. In which case we only need one array and use the index into the array as the subject code. But I have stayed with using two arrays for this example. Possibly the subject codes are not contiguous. (even more likely that students are allowed to take up to ten courses out of dozens or even hundreds FG)

I have kept the test bed program simple for the purposes of this article, but in a real test bed it would be important to test all the 'boundary conditions' that a class could be expected to deal with. One important condition here would be, what happens when we add an eleventh grade to the student? We have an array of ten elements, adding the eleventh takes us into that region of darkness and despair known as "undefined behaviour"... I mentioned at the start of the article that the class specification gives us no clue as to what context it would be used in, and this is an important factor in deciding how to handle boundary conditions such as adding too many elements.

In this case, I can see five possible ways of handling the situation:

Lets ignore the first... If this class were to be used in the context of a library, or as part of a larger, more complex project, then (2) may be the best choice. The last three depend on knowledge of the problem domain and would make an excellent exercise for the reader.

Other things to be dealt with are things like, can we accept a negative score, does the course code need to be within a particular range, how we deal with situations where these constraints are not met?

typedef std::vector<std::pair<subjcode_t, grade_t> > Student;

Student s1;
s1.push_back( std::make_pair( 2, 12 ) );

#include <iostream>
#include <vector>
#include <utility>
const int MAX_ENTRIES = 10;
typedef int subjcode_t;
typedef int score_t;
typedef std::vector<std::pair< subjcode_t, score_t> > subjectlist;
typedef std::vector<std::pair< subjcode_t, score_t> >::iterator subjectiter;
class Cstudent {
public:
  CStudent(){}
  CStudent( subjcode_t g, score_t s );
  int add( subjcode_t g, score_t s );
  void output( std::ostream& os );
private:
  subjectlist subjects;
};

#include "newstudent.h"
CStudent::CStudent(subjcode_t g, score_t s){
  add(g, s);
}
int CStudent::add( subjcode_t g, score_t s ){
  if( subjects.size() >= MAX_ENTRIES ) {
    return MAX_ENTRIES;
  }
  subjects.push_back(std:: make_pair( g, s ));
  return subjects.size();
}
void CStudent::output( std::ostream& os ){
  os << "Student: number entries: = " 
    << subjects.size() << std::endl;
  subjectiter iter = subjects.begin();
  for( ; iter != subjects.end(); iter++ ) {
    os<<"Grade: " <<(*iter).first 
      <<" Score: " << (*iter).second 
      << std::endl;
  }
}

std::map< subjcode_t, grade_t>

std::vector<grade_t>