Review of Code C++

I feel that much of the code is to par for a homework assignment. Yes, I'm being upfront about this being for school. I'm not asking for help, only a review of the code and any helpful suggestions for progressing my learning. It is 46 lines long.
My main concern is why line 38 does not repeat the options. I'm sure it's a simple answer but I'm not just not seeing it. Thank you in advance for your time in viewing the code as well as reading this lengthy explanation.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
  #include <iostream>

using namespace std;
int main(void) {
  char selection = ' ';
  string name = "";
  string address = "";
  cout << "Please enter your name:  ";
  getline(cin, name);
  cout << "Hello " + name << endl;
  cout << "Please enter your address." << endl;
  getline(cin, address); {
    cout << "Please select from the items below" << endl;
    cout << "Items: " << endl;
    cout << "1 - 2021 Weekly Planner" << endl;
    cout << "2 - Cosco 091524 Box Cutter" << endl;
    cout << "3 - Trogonic TE1 Wireless Earbuds" << endl;
    cout << "4 - Exit " << endl << endl;
    cout << "Enter selection: ";
    std::cin >> selection;
    switch (selection) {
    case '1':
      cout << "2021 Weekly Planner will be shipped to " + address << endl;
      cout << "Thank you " + name << "!" <<endl;
      break;
    case '2':
      cout << "Cosco 091524 Box Cutter will be shipped to " + address << endl;
      cout << "Thank you " + name << "!" <<endl;
      break;
    case '3':
      cout << "Trogonic TE1 Wireless Earbuds will be shipped to " + address << endl;
      cout << "Thank you " + name << "!" <<endl;
      break;
    case '4':
      cout << "Thank you " + name << "!" <<endl;
      break;
    default:
      cout << "Invalid selection, please try again.";
    }

    cout << endl << endl;
  }

  while (selection != '4');
  return 0;
}
Line 12, after the std::getline() and before the { add the keyword do.

Now you have a do-while loop instead of a no loop with a stray while statement that does nothing.

Oh, and you might want to add #include<string> too. std::getline is a part of the C++ string library. Some compilers implicitly include it with <iostream>, others don't. MSVC is one of the "don't."

There is no need for main to have void as a parameter with C++. That is something from C IIRC.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
#include <iostream>
#include <string>

int main()
{
   std::cout << "Please enter your name: ";
   std::string name;
   std::getline(std::cin, name);

   std::cout << "Hello " + name << '\n';
   std::cout << "Please enter your address.\n";
   std::string address;
   std::getline(std::cin, address);
   std::cout << '\n';

   char selection = ' ';
   
   do
   {
      std::cout << "Please select from the items below\n";
      std::cout << "Items:\n";
      std::cout << "1 - 2021 Weekly Planner\n";
      std::cout << "2 - Cosco 091524 Box Cutter\n";
      std::cout << "3 - Trogonic TE1 Wireless Earbuds\n";
      std::cout << "4 - Exit\n\n";
      std::cout << "Enter selection: ";
      std::cin >> selection;
      switch (selection)
      {
      case '1':
         std::cout << "2021 Weekly Planner will be shipped to " + address << '\n';
         std::cout << "Thank you " + name << "!\n";
         break;

      case '2':
         std::cout << "Cosco 091524 Box Cutter will be shipped to " + address << '\n';
         std::cout << "Thank you " + name << "!\n";
         break;

      case '3':
         std::cout << "Trogonic TE1 Wireless Earbuds will be shipped to " + address << '\n';
         std::cout << "Thank you " + name << "!\n";
         break;

      case '4':
         std::cout << "Thank you " + name << "!\n";
         break;

      default:
         std::cout << "Invalid selection, please try again.\n";
      }

      std::cout << '\n';
   }
   while (selection != '4');
}
using namespace std is considered bad practice. you can read many, many posts about that on this site and the web.

factor redundant logic and items to keep the code managable, if possible. The top 4 cases all do exactly the same thing if, if you consider wire it so that for case 4 the top print statement does nothing but a blank line. If the assignment is to write a switch statement, then even if its wrong, you write a switch statement. If the assignment is to produce code that does X requirements that can only be seen when running the code, maybe not :)

vector <string> outputtext {"2021 Weekly Planner will be shipped to ", "Cosco 091524 Box Cutter will be shipped to ", "Trogonic TE1 Wireless Earbuds will be shipped to ", "" };
if(selection < 5 && selection > 0)
{
if(selection<4)
cout << outputtext[selection] + address << '\n';
std::cout << "Thank you " + name << "!\n";
}
else
std::cout << "Invalid selection, please try again.\n";

you can do that in the switch too:
case 1: case 2: case 3:
cout << outputtext[selection] + address << '\n';
case 4:
std::cout << "Thank you " + name << "!\n";
break;
default: ...
Last edited on
Thank you both for the quick responses and the assistance. The assignment was only to produce a code where the user would input their name and address and select a product. I am sincerely grateful for this assistance.
Following up on one of the responses above, why is "using namespace std" considered bad practice?
> why is "using namespace std" considered bad practice?

It is considered bad practice to write using namespace directives in a header file
or before an #include in an implementation file.

CoreGuidelines:
Don’t write using namespace at global scope in a header file

Reason: Doing so takes away an #includer’s ability to effectively disambiguate and to use alternatives. It also makes #included headers order-dependent as they may have different meaning when included in different orders.
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rs-using-directive


C++ Coding Standards (Alexandrescu and Sutter):
Don't write namespace usings in a header file or before an #include.

Summary
Namespace usings are for your convenience, not for you to inflict on others: Never write a using declaration or a using directive before an #include directive.

Corollary: In header files, don't write namespace-level using directives or using declarations; instead, explicitly namespace-qualify all names. (The second rule follows from the first, because headers can never know what other header #includes might appear after them.)



About writing using namespace std ; at global scope in an implementation file, there are different shades of opinions.

CoreGuidelines:
Use using namespace directives for transition, for foundation libraries (such as std), or within a local scope (only)

Reason: using namespace can lead to name clashes, so it should be used sparingly. However, it is not always possible to qualify every name from a namespace in user code (e.g., during transition) and sometimes a namespace is so fundamental and prevalent in a code base, that consistent qualification would be verbose and distracting.
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rs-using


C++ Coding Standards (Alexandrescu and Sutter):
In short: You can and should use namespace using declarations and directives liberally in your implementation files after #include directives and feel good about it. Despite repeated assertions to the contrary, namespace using declarations and directives are not evil and they do not defeat the purpose of namespaces. Rather, they are what make namespaces usable.


LLVM Coding Standards:
In implementation files (e.g. .cpp files), the rule is more of a stylistic rule, but is still important. Basically, using explicit namespace prefixes makes the code clearer, because it is immediately obvious what facilities are being used and where they are coming from. And more portable, because namespace clashes cannot occur between LLVM code and other namespaces. The portability rule is important because different standard library implementations expose different symbols (potentially ones they shouldn't), and future revisions to the C++ standard will add more symbols to the std namespace. As such, we never use 'using namespace std;' in LLVM.



My own personal preference is to avoid using namespace directives outside limited local scopes; I find it is easier to read and maintain code that uses qualified names.
Registered users can post here. Sign in or register to post.