Any suggestions?
Yes, quite a few.
First, if you encounter an error you cannot fix yourself and decide to ask for help, please be sure to post code that reproduces your problem. If you cannot figure out where the error comes form, you are not likely to decide correctly which parts of your program are relevant to the error and which are not. My usual approach to this is to start with a new file and add code until the error reappears. I avoid copy and pasting in favor of retyping since this will avoid replicating silly typos. Often times, you will learn how to fix your problem yourself by trying to create such a minimal working (or, failing, in this case) example. Try to avoid non-standard dependencies in your example if they are not relevant to your question. In your example, the clrscr
function should be removed. Also, it would be nice to take the two extra minutes to format your code in a readable way.
Now to your code: I have created saied minimal working example.
#include <cstddef>
#include <cstdlib>
#include <cstring>
#include <iostream>
struct pharmacy_personnel
{
pharmacy_personnel(const char *const name_of_medicine = "Tonic Water")
{
std::strncpy(this->name_of_medicine_, name_of_medicine, 50);
this->name_of_medicine_[49] = '\0';
}
const char *
get_name_of_medicine() const
{
return this->name_of_medicine_;
}
private:
char name_of_medicine_[50];
};
int
main()
{
const std::size_t total = 10;
char name[50];
//clrscr();
std::cout << "Please enter the name of the medicine: ";
std::cin >> name;
pharmacy_personnel * ob2 = new pharmacy_personnel[total];
for (std::size_t i = 0; i < total; ++i)
{
if(std::strcmp(name, ob2[i].get_name_of_medicine()) == 0)
{
std::cout << "Yes, we have that medicine.\n";
return EXIT_SUCCESS;
}
}
std::cout << "Sorry, we don't have that medicine.\n";
return EXIT_FAILURE;
}
“All of a sudden” it compiles without error. This indicates that your original question is missing relevant information. My best bet is that your implementation of pharmacy_presonnel::get_name_of_medicine()
returns a char
when it should return a const char *
.
But there are many other things about this code that really need fixing.
The first surprise might be this one:
$ ./a.out
Please enter the name of the medicine: Tonic Water
Sorry, we don't have that medicine.
The second probably this one (hope you discover it before an attacker does):
$ ./a.out
Please enter the name of the medicine: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
Segmentation fault
Both problems originate from you not using C++ techniques but unsafe low-level C-style I/O. They can surely be fixed with C-style programming but then, why use C++ at all?
In C++, we usually store strings in the std::string
class. This class will take care of allocating a buffer (just) large enough for the string and deallocating it when the object goes out of scope. Then, really, we should also use C++ strings in the pharmacy_personnel
class.
Another nice thing about std::string
is that you can compare them simply using operator ==
. This makes the cstring
header redundant.
The normal “input operator” >>
stops at white space. Since the name of a medicine may very likely contain a blank, we'd better use the std::getline
function to read in an entire line of input.
Applying all this we get:
#include <cstddef>
#include <cstdlib>
#include <iostream>
#include <string> // This is the C++, not the C header!
struct pharmacy_personnel
{
pharmacy_personnel(const std::string& name_of_medicine = "Tonic Water")
: name_of_medicine_(name_of_medicine)
{
}
std::string
get_name_of_medicine() const
{
return this->name_of_medicine_;
}
private:
std::string name_of_medicine_;
};
int
main()
{
const std::size_t total = 10;
std::string name;
std::cout << "Please enter the name of the medicine: ";
std::getline(std::cin, name);
pharmacy_personnel * ob2 = new pharmacy_personnel[total];
for (std::size_t i = 0; i < total; ++i)
{
if(name == ob2[i].get_name_of_medicine())
{
std::cout << "Yes, we have that medicine.\n";
return EXIT_SUCCESS;
}
}
std::cout << "Sorry, we don't have that medicine.\n";
return EXIT_FAILURE;
}
This fixes the string-related issues but there are still others. I'm not quite sure what the purpose of comparing name
with total
identical strings should be but use of the “naked” operator new
is almost certainly a bad idea. (The programs posted so far both have a memory leak for that very reason.) We shall upgrade to C++'s std::vector
.
// Other headers as before.
#include <vector> // for std::vector
// No changes to struct pharmacy_personnel.
int
main()
{
const std::size_t total = 10;
std::string name;
std::cout << "Please enter the name of the medicine: ";
std::getline(std::cin, name);
std::vector<pharmacy_personnel> ob2(total);
for (std::size_t i = 0; i < ob2.size(); ++i)
{
if(name == ob2[i].get_name_of_medicine())
{
std::cout << "Yes, we have that medicine.\n";
return EXIT_SUCCESS;
}
}
std::cout << "Sorry, we don't have that medicine.\n";
return EXIT_FAILURE;
}
If you are already using C++11, we can make the code even more expressive (and less error-prone) by using range-for loops. This frees us from the need to explicitly number the vector elements. (Using iterators, you need not do that in C++98 either, but the new syntax is much cleaner. I have never found that using iterators manually makes for more readable code.)
for (const auto& iter : ob2)
{
if(name == iter.get_name_of_medicine())
{
std::cout << "Yes, we have that medicine.\n";
return EXIT_SUCCESS;
}
}
The last thing I'd like to mention is that your naming and class design could probably be improved. What should the name ob2
tell me? More important: Why does the class pharmacy_personnel
(supposed to represent an employee?) have the name of a medicine associated to it? This would eventually make sense if every employee was responsible for exactly one medicine but that sounds strange. Ideally, your classes model your domain (that is, real-life) as close as possible. Aiming to do so will make your code better structured, more comprehensible and thus easier to maintain and – most important – more fun to code.