Thread: Need some "CLASS-Y" C++ Advice from a person with the time to truly explain ideas.

Originally Posted by C_User

This code runs unless the commented out "define" statements are restored to code status. I'd like to know either how to fix the problems that develop when I do that, or why adding the "define" statements isn't the proper way to approach the class header file.

It looks like you attempted to write header inclusion guards. The name used for the guard should not be the same as the class name. There are few ways to come up with the name, but the important part is that it needs to be unique, e.g., you might have:

Code:

#ifndef C_USER_AISUBJECT_H
#define C_USER_AISUBJECT_H

// ...

#endif

It looks a bit confusing though: your comment for that file starts with "AINOW.cpp", but it looks like that is supposed to be the header file"aisubject.h". Also, if that is indeed a header file, then you should get rid of:

Code:

using namespace std;
using std:: string;
using std:: cout;
using std:: endl;
using std:: cin;
using std:: vector;

Such using directives and using declarations have no place at file scope in a header file.

Oh, and those function definitions should either be declared inline or moved to a source file.

also stop using goto. goto is bad. if you use goto, no one will ever take you seriously as a programmer. use proper loop constructs.

You are probably looking at inclusion guards. Remember that #defines just causes the preprocessor to replace whatever it finds with what is written, e.g. #define whatever test will replace all "whatever" the preprocessor finds with "test". Since those defines are named the same as your class, obviously the preprocessor will replace those, causing compile errors. Therefore, they should not be named the same. For many compilers, you can also just do #pragma once. It's not standard, but it's as close to a standard "non-standard" inclusion guard you will get. It works across VC++, Clang and GCC to my knowledge.

You are abusing the fact that AI_Subject is a class by making all members public. That defies the purpose of a class. A class should hide its member variables and provide functions that manipulates its state. Consider it like a car. A car allows you to use a gas pedal, steering wheel, etc, but does not allow you to mess with the engine directly.

Command_Execute is a free function, so there is no "COMMAND_VAL" or "TARGET_VAL". Possibly you intended this to be a member function? Btw, avoid upper case for variables. Not only does it look ugly, but it's usually reserved for macro names (to avoid clashes with stuff such as variables).

MAP_TARGETS function - again, avoid upper case. Again, TARGET_VAL is not defined. Same for MAP_COMMANDS.

Note: I changed the lines of header containing AI_Subject class to #define AI_Subject_H and eliminated the using std references. Now the compiler is not recognizing the two strings that are part of AI_WORD structure. Looks like I did need the std string ref.

Okay, Googled and found descriptions of comments. I think I can take it from here.

Comments still welcome!

THANKS!

Originally Posted by C_User

"Oh, and those function definitions either be declared in-line or moved to a source file." - C Witch (I don't really recall what "inline" means anymore, although I heard the word a lot in a class a decade and a half ago. Why wouldn't I want the functions bundled in a class that is relevant?)

Your functions are right now not declared inline. If you put them in a header and include in more than one .cpp file, you will get a compile error saying multiple declarations of the same function.
To fix this, you either need to:
- Leave the function prototype in the header and move the definition to a source file, or
- Add the keyword "inline" at the start before the return type to declare the function inline.
A word of warning, though. For the second solution, make absolutely, positively 100% sure that there is only ONE function with that name and that it is ALWAYS the same function. This is usually not hard to do, but don't go put in defines in your header files that allows for conditional compilation. If different .cpp files see a different inline function, then all hell breaks loose.

More on Inclusion Guards please. I need to understand this better. I had though I wanted the Inclusion Guard name to be same as class name to keep compiler from confusing my class with some other code using the same name and producing errors. Now you say I don't. Doesn't that defeat the purpose of the guard? (Not challenging you, but not understanding, either. Thanks for any insights.)

A.hpp declares class "foo".
B.hpp declares class "bar" which depends on class "foo", so it also includes A.hpp.
A.cpp uses both "foo" and "bar", so naturally it includes both A.hpp and B.hpp. Now you get multiple declaration errors because you've suddenly got two "foo".
To avoid this, we use inclusion guards that makes it so that when the compiler sees a header file a second or more times in a source file, it will see an "empty" header file, so that its declarations don't get repeated.
To this end, you can declare a #define, which is named just about anything, as long as it is unique. If it declared the first time a header is included, then the compiler will know it exists the second, etc, time it is included, and therefore, you can avoid including it more than once. The name can be anything.

Comment on "GoTo" - Yes, lots of people say that. To all CS students: "GoTo" is bad according to all sources, but if it works and you just want to be able to test code...

Poor excuse. Learn just to avoid goto in the first place. Then you won't have this problem.

Finally, how is "Command_Execute" a free function? I intended to leave it in class AI_Subject.

Oh, actually, I see your indentation is playing tricks here. Indent properly!

Originally Posted by C_User

Note: I changed the lines of header containing AI_Subject class to #define AI_Subject_H and eliminated the using std references. Now the compiler is not recognizing the two strings that are part of AI_WORD structure. Looks like I did need the std string ref.

No, what you need is to prefix the types with the actual namespace, e.g. std::string.

Originally Posted by C_User

I don't really recall what "inline" means anymore, although I heard the word a lot in a class a decade and a half ago.

In C++, it is a keyword that hints to the compiler that the function may be a good candidate to be inlined, i.e., the compiler, if it accepts the suggestion, will transform the code of the function such that it is inserted at the various points where the function is called, thus eliminating function call overhead. Besides using this keyword, a member function that is defined within the class definition is also said to be declared inline.

Originally Posted by C_User

Why wouldn't I want the functions bundled in a class that is relevant?

A header file would typically be included in more than one source file. Thus, if you define a function in a header file, then the function would be defined in multiple places, which would be an error (the "one definition rule"). An exception to this rule is made for functions that are declared inline since the compiler needs the function definition available in order to do the inlining (whether the function is actually inlined or not does not matter).

Hence, the general approach is to declare functions in header files and define them in source files.

Originally Posted by C_User

More on Inclusion Guards please. I need to understand this better. I had though I wanted the Inclusion Guard name to be same as class name to keep compiler from confusing my class with some other code using the same name and producing errors. Now you say I don't. Doesn't that defeat the purpose of the guard?

The name must be unique. That's the rule in order for the inclusion guard to work. So, if you write something like this:

Code:

#ifndef X
#define X

class X
{
    // ...
};

#endif

Then according to the macro expansion (X is defined to be empty), your code boils down to:

Code:

class
{
    // ...
};

which isn't what you want. Hence, you should use some other name for the header inclusion guard.

Originally Posted by C_User

but if it works and you just want to be able to test code...

Then use a debugger, make an assertion, throw an exception, and/or write automated tests. There may be some cases justifying the use of goto in C, but in C++ other constructs are available. Yet, even in C, the way that you are using goto here looks like a classic case of spaghetti code. The excuse that you just want to be able to test code is utter rubbish: what happens when you're done with the testing? If you rework the code to do without the goto, then you have to test again... without the goto. So, you should have written well structured code without the goto to begin with.

Originally Posted by C_User

Finally, how is "Command_Execute" a free function? I intended to leave it in class AI_Subject.

Oh! Now I understand. Your code is badly indented. All your member functions are defined within the class definition, but because you failed to indent them, they look like non-member function definitions.

Originally Posted by C_User

(How would I link a source file to a header file if the source file contains all the functions I need?)

You don't. Basically, all you need is the code inside the source files. You compile them all and they're bolted together into a program.
However, there is one problem. The compiler will refuse to compile your code if it cannot find the functions you are trying to call (e.g. they are in another source file). So you have to tell the compiler these functions exist. And when multiple files needs to know these functions exists, you can see the code duplication problem. So what we do is put these declarations in a header file which we "paste" into each file that requires to call these functions.

As for the "goto" statements, I got started using BASIC (BASIC, then QuickBASIC). Terms like spaghetti code never make a lot of sense to me when used relative to straightforward "goto" statements such as I've used, although I'm sure that there are a lot of abusive instructors out there who have given "goto" a bad name on the grounds that "there is always a better way". There may always be "another way", but there would have to be a sound basis for the "spaghetti code" reference for me to buy into that overused mode of abuse.
(In answer to your question, "goto" is faster when you've suddenly realized that you need to loop, and you're used to "goto" statements due to a lot of related, early experience in BASIC over the course of many years decades ago, so finding the right location for a "goto" comes naturally, as does tracing the loop, given the obvious nature of the "goto" statement and a named line with a colon that it references.)

It is also equally natural for one who uses loops to know what loop to put where. The point is, that instead of relying on goto, you should learn to rely on loops. Then it will start becoming natural to always use loops instead of having to rely on goto and you won't have this problem in the first place.

Originally Posted by C_User

I'll have to remember to "inline" my functions in header files.

You're already doing that because you defined those member functions within the class definition. The problem is that you did not indent them with respect to the class definition, so they looked like they were non-member functions.

Originally Posted by C_User

As for the "goto" statements, I got started using BASIC (BASIC, then QuickBASIC). Terms like spaghetti code never make a lot of sense to me when used relative to straightforward "goto" statements such as I've used, although I'm sure that there are a lot of abusive instructors out there who have given "goto" a bad name on the grounds that "there is always a better way". There may always be "another way", but there would have to be a sound basis for the "spaghetti code" reference for me to buy into that overused mode of abuse.

Have you tried using loop constructs and breaking out code into functions that do one thing and do it well?

The sound basis here is abstraction and structured code. The use of goto (especially with poorly named labels such as AIASK and xxx) lacks the abstraction provided by a loop construct or a function, therefore it does not provide the mental aid to help the reader understand the code. The use of goto can be well structured (your goto with the AIASK label qualifies, and the other one arguably does so too, especially once you simplify the YesNo check), and the use of a loop construct can be poorly structured, but the normal semantics that come with a loop construct suggests a structure that the more free form goto statement does not.

Originally Posted by C_User

In answer to your question, "goto" is faster when you've suddenly realized that you need to loop, and you're used to "goto" statements due to a lot of related, early experience in BASIC over the course of many years decades ago, so finding the right location for a "goto" comes naturally, as does tracing the loop, given the obvious nature of the "goto" statement and a named line with a colon that it references.

The obvious nature of the goto statement is that it is an unconditional jump, i.e., it is not obvious if it is a loop at all. A loop construct is effectively a goto statement with a label, but with a named pattern (i.e., the loop) imposed along with syntactic limitations, as well as the introduction of an inner scope. So, finding the right location for a goto comes (almost) as naturally to me as it does to you. The difference, therefore, is not whether it is faster to write, but whether it is faster to read and understand.

In the preface to the first edition of Structure and Interpretation of Computer Programs, Abelson and Sussman wrote:

We want to establish the idea that a computer language is not just a way of getting a computer to perform operations but rather that it is a novel formal medium for expressing ideas about methodology. Thus, programs must be written for people to read, and only incidentally for machines to execute.