### Bracketing Search Question

Hello all. I'm on my third day of teaching myself C++, so if you see any glaring issues with this code please feel free to correct me. I'd rather learn the right way over trying to break bad habits later. This program is my rendition of a problem where the computer attempts to guess a number that the user is thinking of between 1 and 100. Everything is working just fine, until you want your number to be 100. The logic I've written isn't capable of getting up to 100, but it gets down to 1 just fine. So I'm not quite sure how to handle the maxnumber. I've tried using an IF statement to toggle a boolean if the computer's guess is 99, and that gave me some odd issues inside the switch. My code in question (I think, at least) lives in case 1. Thanks for any help!

 ``12345678910111213141516171819202122232425262728293031323334353637383940414243444546474849505152535455565758596061626364656667686970717273747576777879808182838485868788899091929394959697`` ``````#include "stdafx.h" #include #include using namespace std; #define LOG(x) cout << x << endl; //Allows me to print to console faster #define DEBUG 1; //Enables some custom debug funcionality #if DEBUG; #define LOG2(x) cout << x << endl; #else #define LOG2(x); #endif int main() { int minNum = 1; //Define minimum and maximum values int maxNum = 100; // std::random_device seeder; //Generates random numbers std::mt19937 engine(seeder()); std::uniform_int_distribution dist(minNum, maxNum); int compGuess = dist(engine); LOG("--------------------------------------------------") LOG("I'd like you to pick a number between " << minNum << " and " << maxNum << "!"); LOG("I'm going to try and guess it. You need to tell me"); LOG("if my guess is too high, or too low."); LOG("--------------------------------------------------"); LOG("Press 0 if my guess is too high."); LOG("Press 1 if my guess is too low."); LOG("Press 4 if my guess is correct.") LOG("--------------------------------------------------"); LOG("Is your number " << compGuess << "?"); bool stillGuessing = 1; int guessCount = 0; while (stillGuessing) { int command; cin >> command; switch (command) { case 0: maxNum = compGuess; int temp2; temp2 = maxNum - minNum; compGuess = temp2 / 2; compGuess = compGuess + minNum; LOG2("Min: " << minNum << " Max: " << maxNum); LOG("Is your number " << compGuess << "?"); guessCount++; break; case 1: minNum = compGuess; int temp; temp = maxNum - compGuess; compGuess = temp / 2; compGuess = compGuess + minNum; LOG2("Min: " << minNum << " Max: " << maxNum); LOG("Is your number " << compGuess << "?"); guessCount++; break; case 4: guessCount++; LOG("Hurray! I have beat you! I guessed your number!!"); LOG("I managed to get your number after only " << guessCount << " guesses!"); stillGuessing = 0; break; default: LOG("You have not entered a correct response. Please review instructions above!"); } } system("PAUSE"); return 0; } ``````
Your macros have a potential problem. You need parentheses around x like this:

 ``123`` `````` #define LOG(x) cout << (x) << endl; ``````

This is because of macro expansion: https://gcc.gnu.org/onlinedocs/cppinternals/Macro-Expansion.html
Thanks for the reply! I like how one of the first sentences in the article you linked states if I don't understand *this*...;

It gives me additional topics to read up on, and stops me doing things the lazy way without fully understanding it. I won't use the #define until I read up on it some more. Thanks again, and if anyone spots anything else please feel free to let me know. Also a nudge in the right direction on how to get my logic to correctly arrive at 100 as a guess.
closed account (48T7M4Gy)
Best do a thorough check but this might be of interest:

 ``1234567891011121314151617181920212223242526272829303132333435363738394041424344454647484950515253545556575859606162`` ``````#include #include int main() { int minNum = 1; int maxNum = 100; int compGuess = 0; bool stillGuessing = true; int guessCount = 0; char command = '?'; // GENERATE FIRST RANDOM GUESS std::random_device seeder; std::mt19937 engine(seeder()); std::uniform_int_distribution dist(minNum, maxNum); compGuess = dist(engine); // MENU std::cout << "--------------------------------------------------\n" << "I'd like you to pick a number between " << minNum << " and " << maxNum << "!\n" << "I'm going to try and guess it. You need to tell me\n" << "if my guess is too high, or too low.\n" << "--------------------------------------------------\n" << "Press H if my guess is too high.\n" << "Press L if my guess is too low.\n" << "Press C if my guess is correct.\n" << "--------------------------------------------------\n"; // USER RESPONSE while ( stillGuessing ) { std::cout << "Min: " << minNum << " Max: " << maxNum << '\n'; compGuess = (minNum + maxNum)/2; std::cout << "Is your number " << compGuess << "? "; std::cin >> command; switch (toupper(command)) { case 'H': maxNum = compGuess - 1; break; case 'L': minNum = compGuess + 1; break; case 'C': std::cout << "Hurray! I have beat you! I guessed your number!!\n"; std::cout << "I managed to get your number after only " << guessCount + 1 << " guesses!\n"; stillGuessing = false; break; default: std::cout << "You have not entered a correct response. Please review instructions above!\n"; } guessCount++; } return 0; }``````
Last edited on
Thank you Kemort for the reply. I see where my maths got a little wonky, I should've stopped trying to over-complicate things and take a step back. Your solution for the math worked beautifully, thanks.
closed account (48T7M4Gy)
My pleasure theoracle09 The changes I made to your program are probably obvious to you:
1. I got rid of the LOG stuff. Note how the <<'s are repeated and using '\n' instead of endl.

2. I put the variables and declarations with initialisation at the start. This leaves a clearer space for the program logic and a single spot to develop the list.

3. I changed the command to a char (upper case conversion) so I didn't have to remember the numerical codes. Laziness and defective attention span or memory is a prime driver for me getting the computer to do the work.

4. The rest of it was an exercise in removing repetition and redundancy. max and min are like a progressively narrowing clamp and possible solutions :)
Last edited on
Kemort, that explanation was exactly what I was looking for. I've taken a step back and am researching the "whys" as opposed to the "hows". More specifically which indentation method to use (Allen), or referencing Google's style guide. Also I'm looking at architecture, instead of flying by the handle and trying to brute force make things work. I'm finding this is explaining a lot more to me than just typing up something random and getting into bad habits.

Thanks for your explanations on why you did those things, it reaffirms the proper way to go about it, and I appreciate it! Doing it the right way the first time has always been a mantra of mine, as opposed to getting into a habit and having to break it later.
I think you both have off-by-one errors in your code. If compGuess is high, then you should set maxNum to compGuess-1, not to compGuess. If compGuess is low then minNum should be set to compGuess+1;
closed account (48T7M4Gy)
 Best do a thorough check

just as well somebody did
theoracle09 wrote:

We can do better than that - Google has specific reasons why it does what it does.

https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#main
https://isocpp.org/faq

JSF Style Guide:
http://www.stroustrup.com/JSF-AV-rules.pdf

Meaningful variable names - comical:
http://www.cplusplus.com/forum/lounge/176684/#msg872881

Extra compiler warning options:
http://www.cplusplus.com/forum/general/183731/#msg899203
Awesome, Theideasman, thanks for those resources! Definitely gives me more to look at.
The better way to write your macro would look like this

 ``123456`` ``````//#define DEBUG 1 #ifdef DEBUG #define LOG2(x) do { std::cout << (x) << std::endl; } while(0) #else #define LOG2(x) #endif ``````

Using a do..while loop and ommiting the closing semicolon makes using it in code better because it causes a compiler error when you forget a semicolon, otherwise bad things can happen.

You shouldn't define DEBUG in the source code. You should either define it at the compiler command line or project settings depending on the compiler. (for Visual Studio it is defined automatically for debug builds IIRC)
Last edited on
IMO, you shouldn't be using macros at all.
@Hydranix, a good rule of thumb is that any time you use a #define you should check for redefinitions.
A little bit safer:
 ``123456789`` ``````# if ! defined LOG2 # if defined DEBUG # define LOG2(x) do { std::cout << (x) << std::endl; } while(0) # else # define LOG2(x) # endif # else # error "LOG2 already has a macro definition" # endif ``````

@TheIdeasMan
Why?

I think a complete ban on the use of macros is a bad idea. Macros have the potential to make your code more expressive and shorter. In my opinion, debugging is one of the most compelling and common use-cases.

Here's one macro I use. The filename and source location will always be correct, it's typesafe, and it can be turned on or off statically with a #define. This is impossible with any other abstraction in C++ unless you do the book-keeping yourself.
 ``12345678910`` ``````# if ! defined LOG_INFO # define LOG_INFO(...) \ do { \ ::logging::write_timestamp(::logging::g_logger); \ ::logging::g_logger.log(" ", __FILE__, ":", __LINE__, ": info: ", \ __VA_ARGS__); \ } while (false) # else # error "LOG_INFO already has a macro definition" # endif ``````

Whose use looks like
`LOG_INFO("pruned branch at, ", branch.cartesian(), ".\n");`[Sat Jan 14 14:04:46 2017]: [5.04636] src/primitive.cxx:1096: info: pruned branch at (240.303, 501.614).

Last edited on
@mbozzi

Like a lot of things, there is advice to beginners, and the opposite to experts. I am sure you have valid reasons and prudently use macros, but tell a beginner it's OK to use macros, they then go out of control and use them everywhere and start using `#define ` for constants.

I am not the only one:
https://isocpp.org/wiki/faq/inline-functions#inline-vs-macros

OTOH I see macros in production code. At the moment I am working on ObjectARX code (C++ for AutoCAD, but for me specifically BricsCAD), they have macros to help create class members. Mind you, there are lots of things I see there where I kind of hide under the hood of my jacket and hold my nose.

closed account (48T7M4Gy)
I think by referring to the OP in spite of all the very illuminating discussion on the pros and cons of macros three points remain:
1. Using a logging macro for normal program output is a little obscure, unorthodox and perhaps even wrong. In my estimation all it did was complicate the code.
2. Especially with programs of this type - short, not processor intensive, relying on user input - if there was any speed difference to be gained, and I doubt there is, it would be negligible.
3. There are more important things to know about with C++ even though macros are obviously useful and they are there to be used when it's appropriate.

@TheIdeasMan
I wouldn't say that I'm an expert by any means! I'm just hesitant to write "never/always do X" because if somebody absorbs some factoid as absolute truth it might remain unquestioned until an opportunity has been missed.

This is true even outside of the technical world, but admittedly it's a lot easier to understand "never do X" than "strongly prefer X except in cases where Y and Z, etc." especially from the perspective of a beginner. C++ is complicated enough already.

For the record, though, I agree with @kemort that the use of macros in the OP is unnecessary at best.

@OP
`#define false true `
Last edited on
@mbozzi

Yes, that is a good way of putting things. There are a bunch of other things which fit into: Beginner, don't do ; someone with good knowledge may be able to do if no other alternative. Some of these include: goto, raw pointers, pointer arithmetic, new and delete, variable names (look at an STL header file), probably some others I don't remember right now.

Also, The OP is 3 days into learning C++, and has already favoured macros - what was the learning work-flow there?
Topic archived. No new replies allowed.