There are multiple ways to do it, and the right way depends on the context. Here are some possible solutions, from the easiest/hackiest to the most verbose/hardest (not an exhaustive listing):
1. Just make everything public
...
struct Card{
public:
Card(string title, string name){
this->title = title;
this->name = name;
}
string title = "Unknown";
string name = "Unknown";
};
...
void showCard(vector<Card> test){
for (it = test.begin(); it != test.end(); it++){
if (it->title != "Unknown"){
printf("%s\n", it->title.c_str());
printf("%s\n", it->name.c_str());
}
}
}
While that does solve the problem, it isn't a good solution. If you would ever want to change the name of member title to main_title you will have quite a pain in doing so, because you will have do edit every single occurrence of title and that can get messy quickly.
2. Make void showCard(vector<Card> test) a friend of struct Card
If void showCard(vector<Card> test) is friend of Card then it will have access to all protected and private members of Card as if they were public. This is a nice solution because only void showCard(vector<Card> test) would have access to these protected members.
Because you can only be friend of previously declared functions, you would need to forward declare the function void showCard(vector<Card> test) before the declaration of Card.
However, because void showCard(vector<Card> test) takes a vector<Card> argument, the class Card needs to be forward declared before the forward declaration of the function.
...
struct Card;
void showCard(vector<Card> test);
struct Card{
public:
friend void showCard(vector<Card> test);
Card(string title, string name){
this->title = title;
this->name = name;
}
protected:
string title = "Unknown";
string name = "Unknown";
};
...
void showCard(vector<Card> test){
for (it = test.begin(); it != test.end(); it++){
if (it->title != "Unknown"){
printf("%s\n", it->title.c_str());
printf("%s\n", it->name.c_str());
}
}
}
3. Create getters and setters for Card
This is one of the canonical implementations. Every time you make a member private/protected you provide a get_member and a set_member methods for it.
That way everyone can access the member, however, they only can access it if they use those methods. You can even create getters/setters for members that don't exist (i.e. you compute them when you need them).
Since the code speaks more than words, here is an implementation:
...
struct Card{
protected:
string title = "Unknown";
string name = "Unknown";
public:
Card(string title, string name){
this->title = title;
this->name = name;
}
string get_title(){
return this->title;
}
void set_title(string new_title){
this->title = new_title;
}
string get_name(){
return this->name;
}
void set_name(string new_name){
this->name = new_name;
}
};
...
void showCard(vector<Card> test){
for (it = test.begin(); it != test.end(); it++){
if (it->get_title() != "Unknown"){
printf("%s\n", it->get_title().c_str());
printf("%s\n", it->get_name().c_str());
}
}
}
If you ever would want to change the name of the member title to main_title, you would only have to edit get_title and set_title and all your code would keep working as if you didn't change it at all. You could even delete that member or do anything else (like fetching it from a database) because the only place where it's existence and name matters is inside get_title and set_title. Without getters and setters, you would need to edit every single occurrence of title in order to do that.
Getters and setters are also wonderful places to improve the const correctness of your code, making it more robust and efficient. A const-correct get/set pair would look something like this:
const string& get_title() const {
return this->title;
}
void set_title(const string& new_title){
this->title = new_title;
}
And a pair for a non-existent member would look like this:
#include <string>
#include <algorithm>
#include <iterator>
string get_title_and_name(){
// Concatenates the title and name
return this->title + " / " + this->name;
}
void set_title_and_name(string new_string){
// Splits the string between a title and a name
std::size_t split_point = 0;
split_point = new_string.find('/');
this->title = new_string.substr(0, split_point);
// We don't want to include the char '/' of
// the new_string in this->name, so use
// (split_point + 1) instead of split_point
this->name = new_string.substr(split_point + 1, new_string.size() - (split_point + 1));
}
While this solution may be more verbose than others, it is also more flexible.
A suggested solution
We could modify solution 3 by creating a new struct Catalog and putting void showCard(vector<Card> test) inside it. This isn't a usual solution, put it open the possibility for we to get rid of some global variables (global variables are almost always evil) and hide the fact that we are using a vector<Card> to keep Cards (we could use a hashmap instead of a vector and that would work as well, so that other code don't need to know which one we chose of the two).
//#include <cstdio>
#include <iostream>
//#include <stdio.h>
#include <vector>
#include <string>
using namespace std;
// As in solution 3
struct Card {
protected:
string title = "Unknown";
string name = "Unknown";
public:
Card(string title, string name){
this->title = title;
this->name = name;
}
// Right now we only need getters,
// but we could have setters as well
// (the names are in camelCase to follow
// showCard() naming convention)
string getTitle(){
return this->title;
}
string getName(){
return this->name;
}
};
struct Catalog {
protected:
// This one was a global variable previously
// Also we don't specify a default value
// for it here, we will do that in the constructor
vector<Card> test;
public:
Catalog(){
// The start value of test will be a empty vector
this->test = vector<Card>();
}
// We moved void showCard(vector<Card> test) to here
void showCard(){
// This is a local variable now
vector<Card>::iterator it;
// For loop as in solution 3
for (it = this->test.begin(); it != this->test.end(); it++){
if (it->getTitle() != "Unknown"){
printf("%s\n", it->getTitle().c_str());
printf("%s\n", it->getName().c_str());
}
}
}
// A new method for adding cards,
// because external code shouldn't care
// about how we add or remove card or even
// if we store cards in this machine or in a web server
void addCard(Card card){
this->test.push_back(card);
}
};
int main()
{
Card book1 = { "Serpent in the heather / Kay Kenyon", "Kay Kenyon"};
Card book2 = { "USA and the Middle East since World War 2 / T.G. Fraser.", "T.G. Fraser"};
Card book3 = { "My Horse and wally", "Jason Weber" };
Catalog catalog;
catalog.addCard(book1);
catalog.addCard(book2);
catalog.addCard(book3);
// We could even do something like
// catalog.addCard({ "My Horse and wally", "Jason Weber" });
// thankfully to the new addCard method.
// We wouldn't even need to declare book1, book2 and book3
// if we used it that way
catalog.showCard();
getchar();
return 0;
}
After you end writing your program, you may be interested in showing it on Code Review in order to receive insights about how other people would approach the same code and learn how people more experienced or with a different background would write such code.
