collapse collapse

* Who's Online

  • Dot Guests: 49
  • Dot Hidden: 0
  • Dot Users: 0

There aren't any users online.

* Board Stats

  • stats Total Members: 88
  • stats Total Posts: 11163
  • stats Total Topics: 1699
  • stats Total Categories: 4
  • stats Total Boards: 76
  • stats Most Online: 248

Author Topic: My First Program  (Read 3013 times)

0 Members and 1 Guest are viewing this topic.

Offline Jake

  • Newbie
  • *
  • Posts: 5
  • Reputation 0
    • View Profile
My First Program
« on: February 18, 2015, 10:13:23 PM »
First, does this forum have a help and support section, or is this all for tutorials? I've got my program all finished and it appears to function as I want it to, I just thought I'd post it so that you guys could chime in and maybe tell me what's bad and what there is to change.

Code: C++
  1. #include <iostream>
  2. #include <vector>
  3. #include <cstdlib>
  4.  
  5. using namespace std;
  6. vector<string> data;
  7.  
  8. string getinput()
  9. {
  10.     string dataline;
  11.    
  12.     string name;
  13.     cout << "Enter Name: ";
  14.     getline(cin, name);
  15.     dataline = "Name: " + name + ", ";
  16.    
  17.     string number;
  18.     cout << "Enter Number: ";
  19.     getline(cin, number);
  20.     dataline = dataline + "Number: " + number + ", ";
  21.    
  22.     string address;
  23.     cout << "Enter Address: ";
  24.     getline(cin, address);
  25.     dataline = dataline + "Address: " + address + " ";
  26.    
  27.     return dataline;
  28. }
  29.    
  30. void printall()
  31. {
  32.     for( vector<string>::const_iterator i = data.begin(); i != data.end(); i++)
  33.     {
  34.         cout << *i << ' ';
  35.         cout << "\n";
  36.     }
  37. }
  38.  
  39. void begin()
  40. {
  41.     system("CLS");
  42.     cout << "Choose a function:\n";
  43.     cout << "1 = Add a new user\n";
  44.     cout << "2 = Print all current users\n";
  45.     string choice;
  46.     getline(cin, choice);
  47.     system("CLS");
  48.     if(choice=="1")
  49.     {
  50.         string input = getinput();
  51.         data.push_back(input);
  52.         cout << "\nUser added successfully!\n";
  53.         system("PAUSE");
  54.         begin();
  55.     }
  56.     else if(choice=="2")
  57.     {
  58.         printall();
  59.         cout << "\n";
  60.         system("PAUSE");
  61.         begin();
  62.     }
  63.     else
  64.     {
  65.         begin();
  66.     }
  67.        
  68. }
  69.  
  70. int main()
  71. {
  72.     begin();
  73.     return 0;
  74. }
  75.  
  76.  

I'm the programming kid from robotics, only one person should know what that means.

Offline Nathan

  • Administrator
  • Hero Member
  • *
  • Posts: 1437
  • Reputation 1768
  • Gender: Male
  • woof woof
    • View Profile
Re: My First Program
« Reply #1 on: February 18, 2015, 10:31:08 PM »
I think we should have a support section - but I'll let Justin (Celestialkey) figure that out.

A couple of things:

- Many people argue global variables are bad form ie:

Code: C++
  1. vector<string> data;

- Shelling out to a system command with system is usually frowned upon because it's platform dependent (arguably there are a lot worse C++ code that you will probably write that will be platform dependent) ie

Code: C++
  1. system("PAUSE");

Check out these links for alternatives:

http://www.cplusplus.com/forum/beginner/106769/
http://www.cplusplus.com/forum/beginner/105772/

You can also replace system("cls") with a number of newlines (as that is what it really does).

- Since C++11 they have introduced the auto keyword - I am a huge detractor but using that keyword is all the rage so this:

Code: C++
  1. for( vector<string>::const_iterator i = data.begin(); i != data.end(); i++)

becomes

Code: C++
  1. for( auto i = data.begin(); i != data.end(); i++)

- Be sure to include <string> - some compilers don't include that by default and will cause compilation errors

- A lot of amateur C++ developers argue against using a global using namespace and insist on fully qualifying it or use it in a smaller scope. Doing this will prevent name/namespace collision - which is pretty rare if you are sticking to STL and standard library.
Projects:
[ Axios Engine ] [ sourcehub ]
Compilers: Microsoft Visual Studio 2008, GNU C++, FASM, MASM, VB 6/.Net.
Languages: C++, PHP, ASM, JS, VB6/.Net, BASIC, HTML, MySQL
Please buy me some books: Amazon Wishlist

Offline Celestialkey

  • Administrator
  • Hero Member
  • *****
  • Posts: 3962
  • Reputation 4874
  • Gender: Male
  • Its Alive!!
    • View Profile
    • www.CelestialCoding.com
Re: My First Program
« Reply #2 on: February 19, 2015, 09:55:11 PM »
The code looks good. It is pretty solid. The only suggestions I can make that Nate didn't already cover are pretty much opinionated, so here are some of my opinions.

Lets start here...
Code: C++
  1.  
  2. void printall()
  3. {
  4.     for( vector<string>::const_iterator i = data.begin(); i != data.end(); i++)
  5.     {
  6.         cout << *i << ' ';
  7.         cout << "\n";
  8.     }
  9. }
  10.  
  11.  


While this works and accomplishes what you want, it is not very readable. Having to dereference a pointer in order to get it's contents is something you should avoid while being a novice IMHO. It causes uneeded congestion and there are simpler ways to accomplish the same thing. One way that is a little more clear is to just reference it directly without an iterator.

Vectors are essentially arrays in memory of whatever type you specified. As such, they can be referenced as if they were arrays, making the below code possible.

Code: C++
  1. for(unsigned i=0; i<data.size(); i++)
  2.      cout << data[i] << "\n";
  3.  

Iterators are good when you have complex classes being pushed onto a stack or into the vector array, but for simple things like this, it is often easier to stick with the simple ways.

Moving along to this function...
Code: C++
  1. string getinput()
  2. {
  3.     string dataline;
  4.    
  5.     string name;
  6.     cout << "Enter Name: ";
  7.     getline(cin, name);
  8.     dataline = "Name: " + name + ", ";
  9.    
  10.     string number;
  11.     cout << "Enter Number: ";
  12.     getline(cin, number);
  13.     dataline = dataline + "Number: " + number + ", ";
  14.    
  15.     string address;
  16.     cout << "Enter Address: ";
  17.     getline(cin, address);
  18.     dataline = dataline + "Address: " + address + " ";
  19.    
  20.     return dataline;
  21. }

Since you are using different variables to store the information, you don't need the many appends. You could instead clean it up into one line similar to below.
Code: C++
  1.     string dataline;
  2.    
  3.     string name;
  4.     cout << "Enter Name: ";
  5.     getline(cin, name);
  6.    
  7.     string number;
  8.     cout << "Enter Number: ";
  9.     getline(cin, number);
  10.    
  11.     string address;
  12.     cout << "Enter Address: ";
  13.     getline(cin, address);
  14.     dataline = "Name: " + name + ", "+ "Number: " + number + ", "+ "Address: " + address + " ";
  15.  

As far as the global namespace goes, as long as it is only used inside the source file, I don't really see an issue with it. I never understood the arguments against it. You only run into issues when placing them in headers or when you find the 2 most unlikely conflicts in defines or naming conventions with non-standard libraries. It is such a rarity that I don't agree they are bad. I do, however, think they should be left outside of headers since you may not always want a scope to be active in all things including it.

As far as a support section goes, I actually thought I had one up. Maybe I haven't shown it to all users yet. I'll check it out.
Created By: Eczuo
Quote
I have noticed that people who claim that everything is predestined, and we can do nothing to change it, look both ways before they cross the road.
Quote
I'd prefer to die standing, than to live on my knees - Che Guevara
Quote
If you change the way you look at something, does that something change in any way?
- Quantum Theory

Hacking
Quote
Never in the field of human conflict was so much owed by so many to so few. - Winston Churchill


Quote from: Revelations 12:4
And his tail drew the third part of the stars of heaven, and did cast them into the earth; and the dragon stood before the woman which was ready to be delivered, for to devour her child as soon as it was born.

Quote
It takes skill to build an empire. It takes an idiot to maintain it.

Offline Jake

  • Newbie
  • *
  • Posts: 5
  • Reputation 0
    • View Profile
Re: My First Program
« Reply #3 on: March 15, 2015, 03:37:24 AM »
I took in a few of these suggestions, and did a little reading. From what I've seen people are saying that using system(anything) uses more resources then just getting it done more pure, so I took a while to find some ways to replace using my system functions, along with adding a remove user function and came up with this.

Code: C++
  1. #include <iostream>
  2. #include <vector>
  3. #include <stdio.h>
  4. #include <string>
  5. using namespace std;
  6.  
  7. string getinput()
  8. {
  9.     string dataline;
  10.    
  11.     string name;
  12.     cout << "Enter Name: ";
  13.     getline(cin, name);
  14.    
  15.     string number;
  16.     cout << "Enter Number: ";
  17.     getline(cin, number);
  18.    
  19.     string address;
  20.     cout << "Enter Address: ";
  21.     getline(cin, address);
  22.     dataline = "Name: " + name + ", Number: " + number + ", Address: " + address + " ";
  23.    
  24.     return dataline;
  25. }
  26.  
  27. void begin(vector<string> data)
  28. {
  29.     printf("\033c"); // This seems to be a nice alternate for clearing the screen, though it relys on console ANSI interpretation.
  30.     cout << "Choose a function:\n";
  31.     cout << "1 = Add a user\n";
  32.     cout << "2 = Remove a user\n";
  33.         cout << "3 = Print users\n";
  34.         cout << "4 = Exit program\n";
  35.     string choice;
  36.     getline(cin, choice);
  37.     printf("\033c");
  38.     if(choice=="1")
  39.     {
  40.         string input = getinput();
  41.         data.push_back(input);
  42.         cout << "\nUser added successfully!\n";
  43.         begin(data);
  44.     }
  45.         else if(choice=="2")
  46.         {
  47.                 if (data.empty())
  48.                 {
  49.                         cout << "No users to display!\n";
  50.                         cout << "\nPress enter to continue...";
  51.                         cin.get();
  52.                         begin(data);
  53.                 }
  54.                 else
  55.                 {
  56.                         for(unsigned i=0; i<data.size(); i++)
  57.                                 cout << i+1 << ". " << data[i] << "\n";
  58.                         int selection = 0;
  59.                         while ((cout << "\nEnter a user's number to remove them... ") && !(cin >> selection))
  60.                         {
  61.                                 cout << "That's not a number; ";
  62.                                 cin.clear();
  63.                                 cin.ignore();
  64.                         }
  65.                         data.erase(data.begin()+selection-1);
  66.                         cout << "Removed " << selection << "!\n";
  67.                         cout << "\nPress enter to continue...";
  68.                         cin.get();
  69.                         begin(data);
  70.                 }
  71.         }
  72.     else if(choice=="3")
  73.     {
  74.                 if (data.empty())
  75.                         cout << "No users to display!\n";
  76.                 else
  77.                 {
  78.                         for(unsigned i=0; i<data.size(); i++)
  79.                                 cout << i+1 << ". " << data[i] << "\n";
  80.                 }
  81.                 cout << "\nPress enter to continue...";
  82.                 cin.get();
  83.         begin(data);
  84.     }
  85.         else if (choice=="4")
  86.                 return;
  87.     else
  88.     {
  89.                 begin(data);
  90.     }
  91. }
  92.  
  93. int main()
  94. {
  95.         vector<string> data;
  96.     begin(data);
  97.     return 0;
  98. }
  99.  

I took an attempt at using a case after so many options, but I guess they only accept integers. I'm also having a little bit of trouble kindly replying to non-integer input, if I enter an entire sentence into cin, it throws back an output for each character. Any suggestions on further cleanup or strategy boosting? I also need to find a way to not remove users when a number that is greater than the total is submitted (Like theres only 5 users, but if I put in 6 it still deletes one of them) I tried to use something like vector.size() or vector.length() but both those seemed to put out a significantly higher value (the amount of characters maybe?)
« Last Edit: March 15, 2015, 03:49:25 AM by Jake »

Offline Nathan

  • Administrator
  • Hero Member
  • *
  • Posts: 1437
  • Reputation 1768
  • Gender: Male
  • woof woof
    • View Profile
Re: My First Program
« Reply #4 on: March 16, 2015, 09:13:00 PM »
Quote
system(anything) uses more resources then just getting it done more pure

Sure - but also keep in mind system("pause") won't work on Mac OS and Linux.

Code: [Select]
@mysql01:~$ pause
bash: pause: command not found
Projects:
[ Axios Engine ] [ sourcehub ]
Compilers: Microsoft Visual Studio 2008, GNU C++, FASM, MASM, VB 6/.Net.
Languages: C++, PHP, ASM, JS, VB6/.Net, BASIC, HTML, MySQL
Please buy me some books: Amazon Wishlist

 

Donate


* Search


* Recent Posts

Image Comparison by Shishka
[May 15, 2017, 01:18:02 PM]


Re: srchub - free source code hosting by Nathan
[December 14, 2015, 11:37:02 PM]


Re: srchub - free source code hosting by Celestialkey
[November 27, 2015, 08:51:42 AM]


Updates by Nathan
[October 30, 2015, 08:27:36 PM]


Re: Client-Server Messaging by Nathan
[October 25, 2015, 05:48:57 PM]