Separating Modules

[HI281]

I separated my program into modules but I got a D on the assignment. Can someone show me what I did wrong? If I can fix it I can resubmit.

  ===============================
    CANDIDATES.CPP
=================================
#include <iostream>
#include <string>
#include <array>


using namespace std;

#include "candidates.h"
#include "states.h"



/**
 * Find the candidate with the indicated name. Returns the array index
 * for the candidate if found, numCandidates if it cannot be found.
 */
int findCandidate (std::string name)
{
  int result = numCandidates;
  for (int i = 0; i < numCandidates && result == numCandidates; ++i)
    if (candidateNames[i] == name)
      result = i;
  return result;
}

/**
 * read the list of candidate names, initializing their delegate counts to 0.
 */
void readCandidatesList ()
{
  cin >> numCandidates;
  string line;
  getline (cin, line);

  for (int i = 0; i < numCandidates; ++i)
    {
      getline (cin, candidateNames[i]);
      delegatesWon[i] = 0;
    }
}



/**
 * For the most recently read primary, determine how many delegates have
 * been won by each candidate.
 */
void assignDelegatesToCandidates ()
{

  int remainingDelegates = delegatesForThisState;
  for (int i = 0; i < numCandidatesInPrimary; ++i)
    {
      int candidateNum = findCandidate(candidate[i]);
      int nDel = (delegatesForThisState * votesForCandidate[i] + (totalVotes-1)) / totalVotes;
      if (nDel > remainingDelegates)
	nDel = remainingDelegates;
      delegatesWon[candidateNum] += nDel;
      remainingDelegates -= nDel;
    }
}


=====================================
CANDIDATES.H
======================================
#ifndef CANDIDATES_H_INCLUDED
#define CANDIDATES_H_INCLUDED
#include <iostream>
#include <string>
#include <array>


const int maxCandidates = 10;
// Names of the candidates participating in this state's primary
string candidate[maxCandidates];

void printPrimaryReport (int candidateNum);

void assignDelegatesToCandidates ();

// Names of all candidates participating in the national election
string candidateNames[maxCandidates];

// How many delegates have been won by each candidate
int delegatesWon[maxCandidates];

// How many candidates in the national election?
int numCandidates;

// How many votes won by each candidate in this state's primary
int votesForCandidate[maxCandidates];

int findCandidate (std::string name);

void readCandidatesList ();


#endif // CANDIDATES_H_INCLUDED
=================================================
STATES.H
=================================================
#ifndef STATES_H_INCLUDED
#define STATES_H_INCLUDED
#include <iostream>
#include <array>

// How many states participate in the election
int numStates;

void readState ();

// How many delegates in the election (over all states)
int totalDelegates = 0;

// How many candidates in the primary for the state being processed
int numCandidatesInPrimary;

// How many votes were cast in the primary for this state
int totalVotes;

// How many delegates are assigned to the state being processed
int delegatesForThisState;



#endif // STATES_H_INCLUDED
========================================================
STATES.CPP
========================================================
#include <iostream>
#include <string>


using namespace std;

#include "states.h"
#include <array>


/**
 * read the info on one state's primaries
 */
void readState ()
{
  totalVotes = 0;
  cin >> numCandidatesInPrimary >> delegatesForThisState;
  totalDelegates += delegatesForThisState;  // "x += y" is a shorthand for "x = x + y"
  string word, line;
  getline (cin, line);
  for (int i = 0; i < numCandidatesInPrimary; ++i)
    {
      cin >> votesForCandidate[i];
      totalVotes = totalVotes + votesForCandidate[i];

      cin >> word;
      getline (cin, line);
      candidate[i] = word + line;
    }
}

============================================
PRIMARIES.CPP
============================================
#include <iostream>
#include <fstream>
#include <string>
#include <array>

using namespace std;

#include "states.h"
#include "candidates.h"


/**
 * Generate the report on the national primary election.
 */
int main(int argc, char** argv)
{
  readCandidatesList();
  int numStates;
  cin >> numStates;
  for (int i = 0; i < numStates; ++i)
    {
      readState();
      assignDelegatesToCandidates();
    }
  for (int i = 0; i < numCandidates; ++i)
    {
      printPrimaryReport(i);
    }
  return 0;
}

/**
 * Print the report line for the indicated candidate
 */
void printPrimaryReport (int candidateNum)
{
  int requiredToWin = (2 * totalDelegates + 2) / 3; // Note: the +2 rounds up
  if (delegatesWon[candidateNum] >= requiredToWin)
    cout << "* ";
  else
    cout << "  ";
  cout << delegatesWon[candidateNum] << " " << candidateNames[candidateNum] << endl;
}

[coder777]

It depends on the requirements whether it is ok or not. So does it fullfill the requirements?

On thing is certainly not good: Having global non const variables in your header files (const like on line 77 is ok). Prefer local variabes and pass them as parameters to your functions.

The problem of global variable in include files is multiple definition error if you include the header more than once and linker problems.

[AbstractionAnon]

As coder777 said, it depends on the requirements.

Since you;re writing in C++, I'm presuming you're taking a C++ class.
If you're indeed taking a C++ class, were you supposed to use classes?
If so, where are your classes?

[HI281]

I was not supposed to change the code at all. I was only supposed to separate them into header and cpp files. coder777, do you mean the #include <string> and #include<array>?

[MikeyBoy]

Isn't it your teacher's job to explain why they gave you a D? Shouldn't you be asking them first, rather than a bunch of people on the Internet?

coder777 is spot on about the global variables, e.g. in states.h you have:

// How many states participate in the election
int numStates;

//...
// How many delegates in the election (over all states)
int totalDelegates = 0;

// How many candidates in the primary for the state being processed
int numCandidatesInPrimary;

// How many votes were cast in the primary for this state
int totalVotes;

// How many delegates are assigned to the state being processed
int delegatesForThisState;

Also, you have some unnecessary header file inclusions. There's no point at all including iostream or array in states.h, because nothing in that file uses them.

In fact, I don't see anything in any of the code that uses array at all - why is it even there?

[HI281]

Here are the instructions for the assignment:

The challenge in this assignment is to divide the program into separately compiled modules (without breaking the already working code).
A Candidates module, consisting of files candidates.h and candidates.cpp, containing code dealing specifically with information specific to individual candidates.
A States module, consisting of files states.h and states.cpp, containing code dealing specifically with the information about each state and the results of the primaries in that state.
The main driver, in the file primaries.cpp, containing code associated with the launch of the program (main) and the sequencing of call to the module’s functions to provide the overall skeleton of the desired report.
There may be some functions and data that are not specifically related to either of these modules, but that contain code needed by them. You should apportion these to the modules in a way consistent with the goals of high cohesion and low coupling.

[HI281]

Well, if I had a decent teacher I would be able to do that but his only answer was "figure it out"

[MikeyBoy]

He told you to figure out why you only got a D? He didn't mark your work or comment on it in any way?

[HI281]

No, he said that programming is self taught.

[dhayden]

Your code doesn't compile:

$ make -k states.o candidates.o primaries.o
g++ -Wall -g -D DEBUG -std=c++11 -c states.cpp
states.cpp: In function ‘void readState()’:
states.cpp:23:14: error: ‘votesForCandidate’ was not declared in this scope
       cin >> votesForCandidate[i];
              ^
states.cpp:28:7: error: ‘candidate’ was not declared in this scope
       candidate[i] = word + line;
       ^

Once you fix this, it still won't run because of the problem that coder777 mentioned. I think that's why you got a D. Generally in a programming class, a program has to compile and run to get anything approaching a good grade.

[coder777]

do you mean the #include <string> and #include<array>?

No, I mean the global variables. Removing unnecessary includes are an optimization but has [usually] no effect on the working code.

However, you can keep your global variable with the keyword extern: In the header file prepend the variables with extern and write the same variable without extern in your cpp:

=================================================
STATES.H
=================================================
#ifndef STATES_H_INCLUDED
#define STATES_H_INCLUDED
#include <iostream>
#include <array>

// How many states participate in the election
extern int numStates;

...


#endif // STATES_H_INCLUDED
========================================================
STATES.CPP
========================================================
#include <iostream>
#include <string>


using namespace std;

#include "states.h"
#include <array>


// How many states participate in the election
int numStates;

...

Note that needs to be done for all global variables.

And as dhayden pointed out, you need to include the header files where they are need. I.e. candidate is declared in what header? -> #include it.