2

I want to make a custom String class for C++. But when I do this:

g++ test.cpp sys/Base.h sys/Base.cpp

I get this error:

sys/Base.cpp: In function 'const char* Base::toChar()':
sys/Base.cpp:57:13: error: 'strval' was not declared in this scope
      return strval;
             ^
sys/Base.cpp: In function 'std::string Base::toStr()':
sys/Base.cpp:60:20: error: 'strval' was not declared in this scope
      return string(strval);
                    ^

test.cpp

#include "sys/Base.h"
int main() {
    Base::write("Hello there.\n");
    return 0;
}

sys/Base.h

// Header file handling
#ifndef ARAVK_BASE_H
#define ARAVK_BASE_H

// Includes
#include <string>

// Global variables
#define EXIT_YAY 0
#define EXIT_ERR 1

using namespace std;

namespace Base {
    // Classes:
        class String {
                static const char* strval;
            public:
                // Constructors:
                    String();
                    String(char[]);
                    String(const char*);
                    String(string);
                // Destructors:
                    ~String();

                // Operators:
                    // =
                        void operator=(const String&);
                        void operator=(const char*&);
                        void operator=(const string&);

                // Conversion:
                    const char* toChar() const;
                    string toStr() const;
        };
    // Functions:
        // Input-Output:
            // Write:
                void write(String);
                void write(string);
                void write(const char*);
            // Read:
                String read();

        // Executing:
            String run(String);
}
#endif

sys/Base.cpp

// Including
#include "Base.h"
#include <string>
#include <stdio.h>
#include <stdlib.h>
#include <cstdio>
#include <iostream>
#include <memory>
#include <stdexcept>

// Global variables
#define EXIT_ERR 1
#define EXIT_YAY 0

/* ------------------------ */
using namespace std;

namespace Base {
    // Classes
        // String functions
            // Constructors
                String::String() {
                    const char* strval = "";
                }
                String::String(const char* str) {
                    const char* strval = str;
                }
                String::String(string str) {
                    const char* strval = str.c_str();
                }
                String::String(char str[]) {
                    const char* strval = str;
                }
            // Destructors
                String::~String() {
                    delete strval;
                }
            // Operators
                // =
                    void String::operator=(const String &strp) {
                        strval = strp.toChar();
                    }
                    void String::operator=(const char* &strp) {
                        strval = strp;
                    }
                    void String::operator=(const string &strp) {
                        strval = strp.c_str();
                    }
            // Conversion:
                const char* toChar() {
                    return strval;
                }
                string toStr() {
                    return string(strval);
                }

    // Functions:
        // Input-Output:
            // Write
                void write(String str)        { printf(str.toChar()); }
                void write(const char* str) { printf(str);              }
                void write(string str)        { printf(str.c_str());  }
            // Read
                String read()                       { char str[100]; scanf("%s", str); return String(str); }
                //TODO: More to come

        // Executing
            /*String run(String command) {
                const char* cmd = command.toChar();
                char buffer[128];
                string result = "";
                std::shared_ptr<FILE> pipe(popen(cmd, "r"), pclose);
                if (!pipe) throw runtime_error("popen() failed!");
                while (!feof(pipe.get())) {
                    if (fgets(buffer, 128, pipe.get()) != NULL)
                        result += buffer;
                }
                return String(result);
            }*/
            String run(String command) {
                char buffer[128];
                std::string result = "";
                const char* cmd = command.toChar();
                FILE* pipe = popen(cmd, "r");
                if (!pipe) throw std::runtime_error("popen() failed!");
                try {
                        while (!feof(pipe)) {
                                if (fgets(buffer, 128, pipe) != NULL)
                                        result += buffer;
                        }
                } catch (...) {
                        pclose(pipe);
                        throw;
                }
                pclose(pipe);
                return String(result);
            }

}  

I'm not sure why this is happening. I think it's related to how I've declared/defined the const char* 'strval'. Can anybody help? P.S: If the answer is too big, this project is on Github: AravK/C-Applications

Arav K.
  • 110
  • 3
  • 12
  • your `String` constructors all declare a local variable that's immediately destroyed. What are you trying to do? – M.M Jun 24 '16 at 09:07
  • your `const char *strval` seems not to be `static` – LibertyPaul Jun 24 '16 at 09:08
  • 1
    There are many things very wrong with your code. This is totally fine being a beginner in C++, but it would be a very broad answer to point out all the issues in your code. That's not really suitable for this site. I would recommend to check out a [good book for C++](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). – Zulan Jun 24 '16 at 09:09
  • 1
    You should not in general list header files on the compilation command line. – davmac Jun 24 '16 at 09:10

2 Answers2

3

Let's take a look at your constructor:

            String::String() {
                const char* strval = "";
            }

This declares a local variable called strval. The variable is local to the constructor; it doesn't exist once execution of the constructor completes.

What you need instead is a member variable - declare it inside the class, but not inside a member method or constructor. In fact, you have already defined it as such in the header file:

    class String {
            static const char* strval;

So, remove the const char * from your constructor and add a class qualifier, so that the line becomes an assignment to the existing variable, rather than creation of a local:

            String::String() {
                String::strval = "";
            }

And also change the return statement that is giving you the error:

              return String::strval;

Or perhaps - and this is likely what you really wanted - remove the static qualifier from the variable definition, and change the constructor instead to just:

            String::String() {
                strval = "";
            }

Furthermore, your destructor incorrectly deletes data that was not necessarily dynamically allocated, or which may belong to another object:

            String::~String() {
                delete strval;
            }

This requires re-working. At the moment the simplest solution is to remove the delete strval altogether.

Your read() function potentially instigates a buffer overflow, by using scanf("%s") with a fixed size buffer and unknown input size:

char str[100]; scanf("%s", str); return String(str);

Finally, your command line:

g++ test.cpp sys/Base.h sys/Base.cpp

... should not include the header file (Base.h). You are specifying the units you want compiled, and Base.h is already included in Base.cpp; it is not a standalone unit that should be compiled invidually.

davmac
  • 20,150
  • 1
  • 40
  • 68
  • Please note that this answer is far from exhaustive. Everything about memory management in the code is absolutely wrong. While I don't expect an exhaustive answer to this question, I think is misleading to provide an incomplete one that just fixes the compilation. – Zulan Jun 24 '16 at 13:34
  • @Zulan if you can point out any specific problems, I'll happily edit them into the answer. (Have put a note in about the destructor - anything else?) – davmac Jun 24 '16 at 13:42
  • Take for example `char str[100]; scanf("%s", str); return String(str);` which combines a buffer overflow with transferring ownership of stack memory. Every single thing about memory is wrong. Those issues are too broad to answer here, but the OP needs to know he should learn more about the basics. – Zulan Jun 24 '16 at 13:52
  • @Zulan ok, I added a note on this as well. I agree that there are a lot of issues, hopefully OP can see that from reading through the problems discussed in the answer. I think your comment on the question is an adequate way of pointing this out, however. – davmac Jun 24 '16 at 14:12
0

yes you did not define the variable in your class as a field. there is 3 locals déclaration in your constructors. just add it the way you have done in the header.

static const char* strval

and remove the définition in your constructors. Just keep the assignement part. regards

claudio06
  • 44
  • 3