-3

I need to clarify why is this error occurring?

char name[50];
clrscr();
cout<<"Please enter the name of the medicine: ";cin>>name;
pharmacy_personnel *ob2;//pharmacy_personnel is a class
ob2 = new pharmacy_personnel[total];
for(int i=0; i<total; i++){
    if(strcmp(name,ob2[i].get_name_of_medicine())==0)
    {
        //code
    }
}

The Error is in the strcmp() function it says:-

argument of type "char" is incompatible with parameter of type "const char *"

Any suggestions?

Niall
  • 30,036
  • 10
  • 99
  • 142
Yashodhan
  • 135
  • 2
  • 10

2 Answers2

2

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.

5gon12eder
  • 24,280
  • 5
  • 45
  • 92
  • Thank you for your tips but I am learning this at school and we are not allowed to use STL like vector also I did not understand the ranged for loops. Can you please explain? – Yashodhan Sep 10 '14 at 22:39
  • [Range-for](https://en.wikipedia.org/wiki/C%2B%2B11#Range-based_for_loop) is a C++11 feature that allows iterating over “ranges” without having to keep track of the begin, current index and end explicitly (the compiler will do it for you). Other languages know this feature as “for-each”. Unfortunately, it is most useful with STL containers but can can also be used with C-style arrays where the length is known at compile-time (see the link). – 5gon12eder Sep 11 '14 at 13:29
0

You are mixing C++ streams (cin, cout) with C-style strcmp. This isn't a good idea, and it's causing you to compare incompatible objects. In particular, the cin>>name doesn't do what you think it does. You probably want name to be a std::string, and to use equality instead of strcmp.

Your code will also be cleaner and more robust if you learn about ranged for loops and std::find. Worth doing now, to have better code later.

Andrew Lazarus
  • 18,205
  • 3
  • 35
  • 53