1

This is my first mini project (I'm intermediate c++ programmer)

I wanted to practice using if statements because I wanted to find the extent of the command, and what I could use it for.

However, throughout my program, I constantly became very annoyed that I'm having to write all this code, to perform a simple task.

The basics of the code is that the user inputs their birth month, and the program outputs their astrology related sign and meaning. My question is, Is there a way, that I could perform the same task, but in less code? Is there a command I could use, or something?

------extra-------------------------------------

In my cs1 class, we recently learned about switch cases. I was thinking that I could use switch cases to fix 1 problem I had, accuracy

Improving the accuracy of the users b-day. Instead of using tons of if statements which can only look for a specific month (or with even more if's month and day) I could use a case that said "1. January 1-20th" However, now this just makes me want to be more accurate about the month. ***Could I possible use more if statements or perhaps something in the case that basically says if the user says <20 then they are Aquarius?

Is there also a different way I could do the program other than switch cases?

    string user_month;
    cout << "This program currently accepts these months in this spelling\n";
    cout << "january, february, march\n\n";

    cout << "Whats the month of your birthday? (lowercase)\n";
    getline(cin, user_month);

    cout << endl;
    cout << user_name << " ... You are about to see your astrological sign and its qualities.\n";
    cout << "Know that they might not be truly accurate and this is just for fun.\n\n";

    if (user_month == "january")
    {
        ;
        cout << endl;
        cout << "You are a Capricorn.\n";
        cout << "Capricorns are usually hardworking, tenacious, diligent, and responsible\n";
        cout << "No matter what the job is, you'll get it done to the best of your ability.\n";
        cout << "Even if you don t have a natural aptitude for a skill, you won t stop working until you ve mastered "
                "it\n.";
    }

    if (user_month == "february")
    {

        cout << endl;
        cout << "You are a Aqaurius.\n";
        cout << "An Aquarius ascribes to a progressive ideology. \n";
        cout << "Because of this you are willing to let go of ";
        cout << "past traditions that no longer serve the people of the future.\n";
        cout << "You aren t content with the answer 'but that s the way it s always been done.' \n";
        cout << "You are willing to try new things, because you know that even if they fail, \n";
        cout << "you know that your getting closer to the right solution.\n";
    }

    if (user_month == "march")
    {

        cout << endl;
        cout << "You are an Aries.\n";
        cout << "At your core, you do what they want and do things your way.\n";
        cout << "You are unafraid of conflict, highly competitive, honest and direct.\n";
        cout << "An Aries is not weighed down by the freedom of choice, and is the sign that is least conflicted about "
                "what they want.\n";
        cout << "You might throw themselves at the world eagerly and without fear.\n";
        cout << "It is one of your most commendable qualities, but also what causes you a great deal of pain and "
                "grief.\n";
    }
    // yes I'm aware my code does not include all the months
    return 0;
}
crash15
  • 37
  • 6
  • 1
    For the purposes of brevity, you could trim your example down to eliminate most of the text. One cout per block is enough for us to get the idea. This is part of the effort you're expected to put in to make your example _minimal_. – Wyck Jan 31 '22 at 15:29
  • 1
    You can put code blocks into **functions**, and then call those functions. That has the benefit of making the calling function more focused on doing *one-thing* rather than having a very large function that does many things. (A function doing *one-thing* is a lot easier to reason about than doing *lots of things*.) – Eljay Jan 31 '22 at 15:31
  • ***we recently learned about switch cases. I was thinking that I could use switch cases to fix 1 problem I had, accuracy*** A switch case does not work directly with strings. You could use a std::map or std::unordered_map to map the month name and the string of text printed for that month. – drescherjm Jan 31 '22 at 15:32
  • you could use a unordered_map to store your key value pairs. The key would be the month and the value being the text you want to print. After defining all the key value pairs those if elses could become just a few lines of code – Dolfos Jan 31 '22 at 15:36
  • welcome to stackoverflow @crash15 I think your question could be more succinct and to the point. Remember, you are asking volunteers to help you here and it takes quite some work to understand what you want. Please take a look at this: https://stackoverflow.com/help/how-to-ask – Benjamin Maurer Jan 31 '22 at 15:38
  • 2
    Given that you are a beginner, practicing "if statements", you can safely assume that there are **many** ways to improve a program like this, which you will learn as you continue learning more C++. Asking if there is a _different way_ to write your very first project may be too broad a question. – Drew Dormann Jan 31 '22 at 15:48
  • @DrewDormann ... So if you were in my place as a beginner, but I still wanted an answer to this question, how would you phrase it? – crash15 Jan 31 '22 at 16:12
  • @crash15 given my understanding of the question - "I'm a beginner - is there a way to write this code that doesn't resemble _beginner code_?" My answer would be [Find a good introductory book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) and finish it. I could redesign your program 20 different ways, but that wouldn't serve you as well as finishing your introductory learning. – Drew Dormann Jan 31 '22 at 16:16

2 Answers2

4

Sometimes, when you have to handle n different values in different ways, you end up with a switch or if-cascade with n branches.

But often, when you perform the same action (with different data) in all branches, there are other ways, e.g., a look-up table.

What are you trying to do? For each month, print a text. So the same action, different data.

Before we look at more complicated solutions, let's improve what you have with the stuff you have learned so far.

First off, you don't need to repeat cout every time, you can chain the stream operator <<:

std::cout << "Hello\n" << "My name is Bob.\n" << "I like potatoes.\n";

Even better, C++ will merge string literals "that are" "adjacent":

std::cout << "Hello\n" 
   "My name is Bob.\n"
   "I like potatoes.\n";

You can also put these large text blocks into variables outside of your main, so you have them out of sight:

const char *capricorn = "You are a Capricorn.\n"
                        "Capricorns are hard...";

So you end up with code like:

string user_month;
getline(cin, user_month);
if (user_month == "january") {
    cout << capricorn;
} else if (user_month == "february") {
    cout << aquarius;
} else if (// etc...

A bit easier to read. Now we see the structure of our program much clearer. We just want to cout something depending on month.

Now, there are things you haven't learned about. E.g., using a "map" (a lookup table):

#include <unordered_map>

// In your function
// Map of key-value pairs
std::unordered_map<std::string, std::string> um = 
{ {"january", "You are a Capricorn\n etc etc\n"} };

// Read month, then:
auto res = um.find(user_month);
if (res != um.end()) {
    // Found in map. res->first is key, res->second is value.
    std::cout << res->second;
} else {
    // Invalid month
    std::cout << "Error: invalid input";
}

Another way would be to store the strings in an array and map the months to array indices (0-11). But you'd need an if-cascade for that too, so maybe that defeats the purpose.

Note that for actual zodiac signs, the task would be a bit more difficult, because they are not about months, but date ranges (e.g., march 21. - april 19.).

Benjamin Maurer
  • 3,602
  • 5
  • 28
  • 49
  • Rather than using `unordered_map`, try using an array of structures, where you use the month number as an index. IMHO, an `unordered_map` is overkill for 12 or 13 items, especially when the data is constant and can be created before the program starts (like the compiler can create the data structure). – Thomas Matthews Jan 31 '22 at 16:32
  • @ThomasMatthews true, but then you still need an if-cascade to match the month names to month index. (i.e., january to 0). My point was about showing a solution without if-cascade. It's not a complete solution by any means anyway. – Benjamin Maurer Jan 31 '22 at 17:02
  • You can search the array for month names. The index of the month name becomes the month index. – Thomas Matthews Jan 31 '22 at 17:27
  • @ThomasMatthews yikes, I don't think that's elegant. At least if you use two separate arrays and the same index across. You could make an array of tuples/structs and look at the first elem as key. But I think that's a premature optimization. A map is for key-value lookups, so it seems like the appropriate data structure. – Benjamin Maurer Feb 01 '22 at 09:18
2

Here's how I would set this up.

I would use a std::map to map from month names to the output you want to associate with it.

std::map::find will perform a lookup and return an iterator for a key-value pair of month and associated output, or an end iterator if it is not found.

A std::optional is a nice way of either having a value or not and it a little less heavy-handed than throwing an exception.

And finally, make good use of functions to describe your program in understandable pieces.

#include <optional>
#include <string>
#include <iostream>
#include <map>

std::map<std::string, std::string> signs = {
    { "january", "You are a capricorn..." },
    { "february", "You are an Aquarius..." },
    // ...
};

std::string getUserInput() {
    std::string input;
    getline(std::cin, input);
    return input;
}

std::optional<std::string> tryGetOutput(const std::string& month) {
    auto it = signs.find(month);
    if (it == signs.end()) return {};
    return it->second;
}

int main() {
    std::string input = getUserInput();
    std::optional<std::string> output = tryGetOutput(input);
    if (!output.has_value()) {
        std::cout << "unknown month" << std::endl;
        return -1;
    }
    std::cout << output.value() << std::endl;
}
Wyck
  • 10,311
  • 6
  • 39
  • 60
  • This program uses C++17 language features. – Wyck Jan 31 '22 at 16:00
  • You don't need `std::map`. You can use a text array with the month number as an index. IMHO, the map is overkill. – Thomas Matthews Jan 31 '22 at 16:29
  • Nice solution, a few comments: (1) For unfathomable reasons, `std::map` is a red-black tree and offers sorted iteration order, but at the cost of slower insertion etc. So I think `unordered_map` should be the default, unless you _need_ to iterate over elements in insertion order. (2) idk if you need a function `getUserInput()`, which turns two lines into five. (3) I love `optional` and FP in general, but might be overkill for a complete beginner (I tried to introduce fewest possible new concepts). (4) `endl` flushes the stream, so `\n` should be default, at least when perf matters. – Benjamin Maurer Jan 31 '22 at 17:08