1

This line canot pass under VS Studio, But it is running under CodeBlocks.

cg1.RegisterGoods("c++", 23, 32);

'void CGoods::RegisterGoods(char [],int,float)': cannot convert argument 1 from 'const char [4]' to 'char []'

like so:

#define _CRT_SECURE_NO_WARNINGS
#include<iostream>
#include <cstring>
using namespace std;

class CGoods
{
private:
    char    Name[21];
    int Amount;
    float   Price;
    float   Total_value;

public:
    void  RegisterGoods(char name[], int amount, float price)
    {
        strcpy(Name,name);
        Amount = amount;
        Price = price;
    }
    void  CountTotal(void)
    {
        Total_value = Price * Amount;
    }
    void  GetName(char name[])
    {
        strcpy(name,Name);
    }
    int GetMount(void)
    {
        return Amount;
    }

    float GetPrice(void)
    {
        return Price;
    }
    float GetTotal(void)
    {
        return Total_value;
    }
};

int  main() {
    CGoods cg1;
    cg1.RegisterGoods("c++", 23, 32);
    cout<<cg1.GetPrice()<<endl;
    cout<<cg1.GetMount();
    return 0;
}
skratchi.at
  • 1,151
  • 7
  • 22
Dan Li
  • 55
  • 7
  • 3
    Please don't pass arrays for string literals, use `const char*` isntead. – Thomas Lang Apr 02 '19 at 08:50
  • 5
    Use `std::string` instead* – Tas Apr 02 '19 at 08:51
  • That might be because CodeBlocks uses different compiler or compiler settings, and does not enforce that. But a string literal like `"c++"` is a `const char[4]` and passing it as `char *` violates const correctness. So this should always be handled as an error, even so it is often only handled as warning by compilers and/or IDEs in their default settings. – t.niese Apr 02 '19 at 09:08

3 Answers3

2

Don't use c-constructs for things, c++ has better answers. char-pointers can lead to unwanted behaviors and nasty buffer overflow exploits and so on. It is far better to use a std::string.

Change your member-function RegisterGoods to:

void  RegisterGoods(std::string const & name, int const amount, float const price)
{
    Name = name;
    Amount = amount;
    Price = price;
}

and your declaration of Nameto:

private:
    std::string Name;

your return function GetName to:

std::string GetName() const
{
    return Name;
}

OR

void  GetName(std::string & name) const
{
    name = Name;
}

also add the include for std::string:

#include <string>

Tip for a better code... don't use using namespace std. std is a enormously huge namespace. Unintentionally you may override a function out of std and you end up with a nearly undebuggable error.

Also define your parameters in setter functions as const, so you can't change the value of it unintentionally.

skratchi.at
  • 1,151
  • 7
  • 22
  • 1
    the function `GetName(void)` could be declared `const` (no need for the return type) – Stack Danny Apr 02 '19 at 08:58
  • @StackDanny included your comment into my answer – skratchi.at Apr 02 '19 at 09:00
  • I mean `std::string GetName() const` instead of `const std::string GetName()` – Stack Danny Apr 02 '19 at 09:02
  • thanks for the edit, but the `const`s for the parameters are not unnecessary. you may correct me, but i refused that change – skratchi.at Apr 02 '19 at 09:17
  • they are unnecessary as the parameters are not send via reference. Beeing send by value copies them anyway. But you can surely leave it `const` as there is one good reason (for good practise) and that is to avoid cases where one would accidentely assign the paramater like discussed [here](https://stackoverflow.com/questions/6125425/const-for-non-reference-arguments). – Stack Danny Apr 02 '19 at 09:21
  • @StackDanny as the names are very similar, I strongly suggest the `const`. So it is impossible to assign them the wrong way around. It is not only good practice. Also, where is the use in letting them change, when the function is a setter? It lets room for hard to debug mistakes. – skratchi.at Apr 02 '19 at 09:36
2

char name[] as a function parameter is equivalent to char *name while your string literal has a type const char [4] which can only be (safely) converted to const char *, so you have to change your parameter like this:

void RegisterGoods(const char *name, int amount, float price)

and here:

 // Renamed to SetName given that it's what this function actually does
void SetName(const char *name)

In general though you shouldn't use plain char arrays to store strings in C++, you should instead prefer using std::string:

std::string Name;
...
void SetName(std::string name)
{
    // take advantage of move semantics to avoid redundant copying
    // if you are using C++11 and beyond
    Name = std::move(name);
}
r3mus n0x
  • 5,954
  • 1
  • 13
  • 34
  • please don't suggest passing a string by-value. better use a const reference. – skratchi.at Apr 02 '19 at 09:09
  • @skratchi.at, actually passing by-value should be a default option if this value is then stored since move-semantics were introduces. Read more [here](https://codesynthesis.com/~boris/blog//2012/06/19/efficient-argument-passing-cxx11-part1/). – r3mus n0x Apr 02 '19 at 09:19
1

Is that means that we don't need strcpy and char in c++ anymore? Because i replaced all the 'char' with string and its funktion also. like this:

    private:
        std::string Name;
    public:
        void  RegisterGoods(const string name, int amount, float price)
            {
              Name=name;
              Amount = amount;
              Price = price;
            }

        const std::string GetName()
        {
            return Name;
        }
Dan Li
  • 55
  • 7