Allowing the user to alphabetize or randomize the contents of a text file

As a newbie programmer, I decided to try my hand at making a program whose function actually might be useful to someone, somewhere.

This utility takes a text file, reads it out to the user, displays its length (in bytes), asks the user if they would like to alphabetize, randomize, or do nothing to the contents of the file. It then asks the user what name they would like to save the new file under, assuming they chose to randomize or alphabetize (with overwrite protection!).

I'm hoping to have you (kindly) pick this program apart and tell me things I ought to do differently for the sake of standards compliance, headache avoidance, etc.

```
#include
#include
#include
#include
#include
using namespace std;

//------ NOTE ------
//Words surrounded with '*' are functions.
//Words surrounded with '|' are variables.

//prototyping
unsigned int getLength(string fileName);
string alphabetize(string source);
string getContent(string fileName);
string scramble(string source, int numOfPasses);
bool doesFileExist(string name);
//all done!

int main()
{
char input; //used throughout main for various user selection purposes.

cout > myFile; //declares then stores the name of the text file you want to scramble/alphabetize.

//------ NOTE ------
//at this point here, add a check to see if the file the user entered actually exists!

cout > input;

if(input =='n'){
cout > input; //there's that char variable again!

if (input == 'a'){
//user decided to alphabetize the file. See alphabetize for how this is done.
Scrambled = alphabetize(getContent(myFile)); //sets |Scrambled| equal to the string returned by alphabetize()

}else if(input == 'r'){
//user decided to randomize the file. See scramble for how this is done.
cout > numPass; //user specifies how many passes the randomizer should perform.

Scrambled = scramble(getContent(myFile),numPass); //sets |Scrambled| equal to the

In this review, I'll omit the code that was not implemented by you. At some point, you can try to figure out how it works and then replace it with your own implementation that can be reviewed here.

As a beginner, it's great to see that you've started using functions to increase modularity. However, you can use more, as main() is still doing a lot of work. You generally should just have it collect input, call other functions, and display something at the end. Most of the other functions will perform one primary thing and usually shouldn't need to display anything. Many of your conditionals in main() can be moved elsewhere, also making it easier to see what main() should really be doing itself.

The structure at this level also seems okay, but more can still be done. For instance, there is probably a better way to alphabetize, without using your own function. It looks like you're just needing to sort the characters, and if I'm not mistaken, you're attempting to implement bubble sort. Either way, this is one of the slowest sorting algorithms around, and should be avoided for serious code like this. Instead of using this function, you can use std::sort() from the standard library, which utilizes a quicker sorting algorithm called quicksort.

std::sort(source.begin(), source.end());

The same applies to scramble(). It looks like you're trying to shuffle the characters in the string, so you can instead use std::random_shuffle() (assuming you don't have access to C++11):

std::random_shuffle(source.begin(), source.end());

(The linked pages should also tell you what begin() and end() are for.)

While it is great that you're trying to implement these things yourself, you should eventually learn how to utilize the standard library to help simplify your code.

I'll now cover some individual points for further simplification:

-
You don't actually need function prototypes. In order to avoid them, simply define main() below the other functions. That way, it'll already be aware of them.

-
It is common for beginners to use std::endl excessively for displaying newlines. What is not always told is that it does something else: flushes the buffer. This is a slower operation by itself, and it's not something that's usually needed.

Instead, you can display newlines with "\n":

std::cout << "some text\n\n";

This is equivalent to the std::endl version:

std::cout << "some text" << std::endl << std::endl;

More info about this can be found here.

-
You can "move" doesFileExist() to main() since file input is involved. But instead of returning and using a bool, you can just terminate from main() if the file cannot be opened:

if (!file)
{
    std::cerr << "file cannot be opened";
    return EXIT_FAILURE;
}

Notes: std::cerr is the error output stream, and EXIT_FAILURE is a macro primarily used for returning a failed error code.

StackExchange Code Review Q#70384, answer score: 9