0

I'm currently writing a program that will help a college keep track of which students are in what clubs. This code will be pasted into existing code that works, as provided by my professor. Could someone please look my code over, and help me work through my mistakes? I included comments to explain what everything should do.

Also, please note this is an assignment and I cannot change anything about the class. I'm also a beginner, so keep it simple, please. Your help will be extremely appreciated beyond all measure.

class Club
{
public:
    Club();
    Club(Club &c);
    Club(string cname);
    void addMember(string name);
    void removeMember(string name);
    string getClubName() const;
    string setClubName(const string& nameOfClub);
    void loadClub();
    bool isMember(string& name) const;
    string getAllMembers() const;
    friend Club mergeClubs(Club& c1, Club& c2);
    ~Club();
private:
    string *members;
    int numMembers;
    string clubName;
};

Club::Club()
{
    clubName = "";
    numMembers = 0;
}

Club::Club(Club &c)
{
    numMembers = c.numMembers;

    members = new string [numMembers];
    for (int i = 0; i < numMembers; i++)
    {
        members[i] = c.members[i];
    }

    //copy constructor
    //watch out for memory leaks

}

Club::Club(string cname)
{
    clubName = cname;

    //cname should be saved as the club's name in the clubName variable
}

void Club::addMember(string name)
{
    string *m;
    m = new string [numMembers];
    string *members = m;
    for (int i = 0; i < numMembers; i++)
    {
        for (int j = 0; j < name.length(); i++)
        {
            m[i] = name[j];
        }
    }

    delete [] m;

    //adds new member to the club
    //records their name in the members variable
    //may need a dynamic array to make this work, watch for memory leaks!

}

void Club::removeMember(string name)
{
    string *m;
    m = new string [numMembers];
    string *members = m;
    for (int i = 0; i < numMembers; i++)
    {
        if ( m[i] == name)
        {
            m[i] = "";
        }
    }

    delete [] m;

    //deletes the person from the array in members
    //will do nothing if the person is not in the array to begin with
    //may require dynamic array to make this work- watch for memory leaks!
    //if the person's name appears more than once, just delete the first instance

}

string Club::getClubName() const

{
    return clubName;
    //getter of clubName
}

string Club::setClubName(const string& nameOfClub)
{
    return clubName = nameOfClub;
    //setter of clubName
}

void Club::loadClub()
{
    //should print "tell me the name of the next member of the club"
    //reads into the variable name
    //uses addMember() to add that person to the club
    //the input should include up to the line break as the name, so it should take in "jane doe" as an entry
    //keeps asking for more names until the user enters a blank entry

    string name;
    do
    {
        cout << "Tell me the name of the next member of the club, ";
        cout << "or submit a blank entry to stopent ering names." << endl;
        getline(cin, name, '\n');
        addMember(name);

    } while (name != "");

}

bool Club::isMember(string& name) const
{
    /*for (int i = 0; i < numMembers; i++)
    {
        for (int j = 0; j < name.length(); j++)
        {
            if (members[i] == name)
            {

                return true;
            }
            else
            {
                return false;
            }
        }

    }*/

    for (int i = 0; i < numMembers; i++)
    {
        if (members[i] == name)
        {
            return true;
        }
        return false;

    }

    //returns true if the person is a member of the club, false otherwise
}

string Club::getAllMembers() const
{
    for (int i = 0; i < numMembers; i++)
    {
        return members[i];
        cout << ", ";
    }
    cout << endl;

    //returns a string of all the names of the members of the club
    //commas and spaces separating every entry of the list
    //should not be a comma following the last name in the list

}

Club mergeClubs(Club& c1, Club& c2)
{
    //creates a new club from 2 existing clubs
    //combined club name should be Club 1/Club 2

    Club temp;
    temp.clubName = c1.clubName + "/" + c2.clubName;
    return temp;
}

Club::~Club()
{
    delete [] members;

    //destructor
    //watch out for memory leaks
}
4
  • 2
    Tl;DR. You'll get more responses if you shorten your query and code. Commented Apr 26, 2014 at 0:00
  • in string Club::getAllMembers() const there is return inside the for the couts will never get called also this returnis seems unwanted ... well my suggestion is : make a specific questions for each specific problem you have... Commented Apr 26, 2014 at 0:07
  • Here's some specific questions: 1. Are each of my constructors correct 2. How do I go about creating dynamic arrays in the two functions that I am allowed to use them in Commented Apr 26, 2014 at 0:17
  • @user3574823 No reason to remove the code? Why? Commented Apr 26, 2014 at 1:03

2 Answers 2

2

For some reason I cannot fathom I have (mostly) corrected this program for you. There are still some nasty things like passing copies of strings into const functions but fixing these would be mere optimisations. At least this program is logically correct.

#include <vector>
#include <string>
#include <iostream>
#include <algorithm>
#include <sstream>

using namespace std;

class Club
{
public:
    Club();
    Club(const Club &c);
    Club(string cname);
    void addMember(string name);
    void removeMember(string name);
    string getClubName() const;
    string setClubName(const string& nameOfClub);
    void loadClub();
    bool isMember(const string& name) const;
    string getAllMembers() const;
    friend Club mergeClubs(Club& c1, Club& c2);
    ~Club();
private:
    vector<string> members;
    string clubName;
};

Club::Club()
: members()
, clubName()
{
}

Club::Club(const Club &c)
: members(c.members)
, clubName(c.clubName)
{
}

Club::Club(string cname)
: members()
, clubName(cname)
{
}

void Club::addMember(string name)
{
    members.push_back(name);
}

void Club::removeMember(string name)
{
    members.erase(remove(members.begin(), members.end(), name), members.end());
}

string Club::getClubName() const

{
    return clubName;
    //getter of clubName
}

string Club::setClubName(const string& nameOfClub)
{
    return clubName = nameOfClub;
    //setter of clubName
}

void Club::loadClub()
{
    //should print "tell me the name of the next member of the club"
    //reads into the variable name
    //uses addMember() to add that person to the club
    //the input should include up to the line break as the name, so it should take in "jane doe" as an entry
    //keeps asking for more names until the user enters a blank entry

    string name;
    do
    {
        cout << "Tell me the name of the next member of the club, ";
        cout << "or submit a blank entry to stopent ering names." << endl;
        getline(cin, name, '\n');
        addMember(name);

    } while (name != "");

}

bool Club::isMember(const string& name) const
{
    return find(members.begin(), members.end(), name) != members.end();
}

string Club::getAllMembers() const
{
    stringstream result;
    vector<string>::const_iterator b = members.begin(), e = members.end();
    for (bool comma = false ; b != e; ++b, comma = true)
    {
        if (comma) {
            result << ", ";
        }
        result << *b;
    }
    return result.str();
}

Club mergeClubs(Club& c1, Club& c2)
{
    Club temp(c1.clubName + "/" + c2.clubName);

    struct memberAdd { 
        Club& _club;
        memberAdd(Club& club) : _club(club) {}
        void operator()(const string& member) {
            _club.addMember(member);
        }
    };

    for_each(c1.members.begin(), c1.members.end(), memberAdd(temp));
    for_each(c2.members.begin(), c2.members.end(), memberAdd(temp));

    return temp;
}

Club::~Club()
{
    //destructor
    //watch out for memory leaks
}

int main()
{
    Club boys("red");
    boys.addMember("Ben");
    boys.addMember("Paul");

    Club girls("blue");
    girls.addMember("Lucy");
    girls.addMember("Hermione");

    Club unisex = mergeClubs(boys, girls);

    cout << unisex.getClubName() << " has the following members: " << unisex.getAllMembers() << endl;

}
Sign up to request clarification or add additional context in comments.

Comments

1

Some initial comments:

  1. The copy constructor should take a const Club& like this:

    Club(const Club& club)

  2. members and numMembers are anachronistic. Use a std::vector for members and drop the numMembers altogether (std::vector has a size() method).

  3. Club::Club does not initialise members or numMembers. This will result in a possible crash in the destrutor. This would be solved if you replaced them with a vector.

  4. the logic in addMember does not make sense.

  5. nor in removeMember

  6. isMember should take a const string& and you would be well advised to write it in terms of std::find() (#include <algorithm>)

  7. I could go on...

5 Comments

I have to use numMembers. Everything in the class is a skeleton provided by my professor and cannot be changed.
Get a new professor. He doesn't know this language.
exactly what I just thought :)))
@user3574823 not putting judgement on the professor's knowledge of the language, for some (possibly nefarious) reason, he clearly doesn't want you using the standard library, which would make this task considerably easier (nearly trivial). That aside, #3 in this list is the most detrimental to your code. Start with that, specifically in reference to Club::Club(string cname).
Hm well I really want to make this code work. Here's some suggestions I have been given, but I'm not clear on how to code this because well, we have never done this before. 1. in Club::Club(const Club &c), I should use std::vector 2. in void Club::addMember(string name), I should push_back() the new member and increment members 3. in Club::removeMember(string name), I should use pop_back() or erase(position) 4. in string Club::getAllMembers() const, I should make a local string and append the names and commas to it

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.