Prev: shared_ptr question
Next: template<class... Arg>; variadic "while" //variadic "for"//
From: Kenneth 'Bessarion' Boyd on 17 Jun 2010 07:41
On Jun 17, 2:16 am, Matt Habel <habel...(a)gmail.com> wrote:
> Ok, so I've just recently started programming again with c++. I've
> been working on a simple text RPG and i need some help with my current
> project.
> And also, this is the first time ive posted on this forum so if i post
> my code wrong or something please tell me.

It is traditional, at this scale of source code, to find some hosting
for the files and just link to the files.

I already see quite a bit of technical commentary (in particular, why
the link error is exactly what it says.)

> #include<iostream>
> #include <stdlib.h>
> #include <time.h>
> #include<stdio.h>
> #include"Function.h"
> #include"globals.h"

*blink*

Obviously leaning towards interactive-fiction for the display. [I
don't see a curses library include here.]

But why do we have both #include <iostream> and #include <stdio.h>?
They're basically the same idea (very low-level interaction with stdin
and stdout). As you're using std::cin and std::cout, #include
<stdio.h> doesn't belong here.

> using namespace std;

Not awesome. I'd do this only if it actually got me significant
source code compression. It's polite to do this only in files that
are never included.

Note that I'm not sold on #include <cstdlib> over #include
<stdlib.h> , etc.
* #include <stdlib.h> isn't going away (regardless of formal
deprecation) as long as compiling C code in C++ is a design objective.
* The only difference is whether the functions are guaranteed to be
declared globally [#include <stdlib.h>], or guaranteed to be declared
in the std namespace [#include <cstdlib>].

> int main()
> {

/* ... */

> if (character = true);
> goto Name;
>
> if (character = false);
> return 0;
>
> if (character != (true) | (false));
> {
> cout<<"Please enter Y or N: ";
> goto Funct1;
> }

As pointed out elsewhere, the above is effectively

character = '\01'; /* character with value 1; ASCII encoding would
be CTRL-B, although I wouldn't count on std::cin actually sending that
through */
goto Name;

You may have gotten a warning about this.

Remember, what's coming in from std::cin are character literals. The
true and false constants won't do anything intuitive here.
{
cin>>character;
while('Y'!=character && 'y'!=character)
{
if ('N'==character || 'n'==character)
return 0;
cout<<"Please enter Y or N: ";
cin>>character;
}
}

Unlike BASIC, C and C++ use operator == to test for equality.
operator = *always* means assignment here, it isn't context-sensitive
(unlike BASIC).

Some style guidelines (including the one I use for internal projects)
recommend putting constants, literals, etc. on the left-hand side of
== specifically to convert typoed = into errors.

/* ... */

Ideally we'd use a single function for all of these 1..n prompts. The
naive return value would be char. Left as exercise for the reader.

> if (selection = 1);
> goto Meadows;
> if (selection = 2);
> goto Fields;
> if (selection = 3);
> goto Stoneroad;
> if (selection = 4);
> goto Dirtroad;
> if (selection = 5);
> goto General1;
> if (selection = 6);
> goto Inn;

Character literals and operator == again. Actually, we can go further
and use a switch operator to completely avoid risk of confusion:

switch(selection)
{
default: /* some sort of unexpected input handling */
case '1': goto Meadows;
case '2': goto Fields;
case '3': goto Stoneroad;
case '4': goto Dirtroad;
case '5': goto General1;
case '6': goto Inn;
}

We also have a massive case of uncontrolled fall-through which could
be fixed by an earlier recommendation to convert all of these gotos to
function calls.

/* ... */
> Meadows:
>
> {
> int randNumb;

Uninitialized, thus undefined behavior for otherwise-working code.
Looking at the following, suspect what's wanted is
const int randNumb = rand()%15+1;

Given what's in battle.cpp, pretty sure this is an oversight.

> {
> if (randNumb = 1 | 2 | 3 | 4 | 5);
> goto meadrat;
> if (randNumb = 6 | 7 | 8 | 9 | 10);
> goto gardsnake;
> if (randNumb = 11 | 12 | 13 | 14 | 15);
> goto badger;
> }

Again, operator == rather than = . This abuse of the bitwise or
operator | cannot be visually matched to any programming language I
have actually tried to use, although ML and Haskell are "closer".

A switch statement with cases 1 through 15 would be overkill, but
would allow adding other encounters with little disruption.

> Thats my main program.
>
> Heres the battle script.

/* ... */

> int battle()
> {
> srand(time(0));

srand(time(0)) belongs in your main(), not here. It seeds the
pseudorandom number generator, and typically is called exactly once.

Others have already adequately addressed globals.h and functions.h.


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Seungbeom Kim on 18 Jun 2010 00:23
On 2010-06-17 00:16, Matt Habel wrote:
> cin>>character;
[...]
> cin>>name;
[...]
> cin>>selection;
[...]
> cin>>select;
[...]

Nobody has pointed this out yet, but you should always check whether
these functions succeeded or failed. When they fail, the variables are
not changed, so reading them is pointless. For example:

if (cin >> x) {
// x has the input; use it.
}
else {
// oops, failed to read the input into x;
// x doesn't have the input.
}

What should be done in case of a failure? Usually, if the reason was
EOF, you cannot expect further input, so you want to abandon any future
tasks that would depend on future input and jump to the closing. For
example, if a simple adder program was reading numbers and met EOF,
the sensible thing to do is to break out of the loop, print the sum
and exit. If the premature EOF is an error, a simple exit(EXIT_FAILURE)
or abort() is also fine.

If the reason was a formatting error (i.e. you expected a number but
the input couldn't be interpreted as one, such as "foo"), then you may
want to clear the input stream state and the input buffer, optionally
print an error message, and retry the reading. Note that this may make
sense if the input is interactive (from a terminal), but may not if
the input is from a redirection or a pipe, in which case there is little
hope for recovery and you may just want to abort() with an error message.

Read the following articles of mine:
http://groups.google.com/group/comp.lang.c++.moderated/msg/05ab7f932cbe68bf
http://groups.google.com/group/comp.lang.c++.moderated/msg/8b366a962e14570a
for some concrete examples.

This is not a theoretical problem but a very practical and important
one; if you don't handle input errors correctly, you end up silently
using garbage values, or pouring out a massive amount of error messages
in an infinite loop before the user has a chance to kill the program.

This sounds complicated, but robust input handling is inherently so.
Too bad that the standard library doesn't provide any function for
simple-but-robust input handling for interactive programs.

--
Seungbeom Kim

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

First  |  Prev  | 
Pages: 1 2
Prev: shared_ptr question
Next: template<class... Arg>; variadic "while" //variadic "for"//