HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcppMinor

Enter values, generate a list, and search

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
searchenterandgeneratevalueslist

Problem

My assignment in school:

  • Read in names to sort until the user types the “enter” key



  • After sorting and displaying the array of strings



  • Do a binary search to find if the string is in the array



  • If the string is in the array, tell the user the index of the string.



  • If the string is not in the array, tell the user that it is missing.



Can this get smaller, faster, and easier to read?

```
#include
#include
#include
#include

using namespace std;

void main ( ) { vector nameTemp { };
vector > nameContainer { };
bool Continue ( true );
char c { };
do
{ / Stay in loop while ( !Continue ) /
do
{ Continue = true;
cout ) */
sort ( nameContainer.begin ( ),
nameContainer.end ( ),
[ ] ( const vector & a, const vector & b )
{ return a 9 ) { cout ( Row );

cout << "\n";
RowNumber++;
}
}
while ( !Continue );

Continue = false;
/ Stay in loop while ( !Continue ) /
do
{ Continue = true;
cout << "\n\tSearch Entries: ";
while ( ( c = cin.get ( ) ) != '\n' )
{
nameTemp.push_back ( c );
Continue = false;
} / Strings make for easy printing /
string sNameTemp { nameTemp.begin ( ), nameTemp.end ( ) };

if ( !Continue )
{ / Active check for No Result /
size_t ColumnNumber { 0 };
bool Result { };
bool noResult ( true );

for ( const auto & Column : nameContainer )
{ / Binary search through 0 index /
bool Result = binary_search ( Column.begin ( ),

Solution

Use functions

Putting all your code in one function is hard to maintain and read. By dividing your code into functions you can name the functions so that the code becomes self documenting.

Includes are illegal

Technically this is not legal:

#include 
#include  
#include 
#include 


The space after ` is part of the name (as defined by the standard). If this is working you are relying on compiler specific behavior.

Using namespace std

This is bad practice (and a bad habbit).

using namespace std;


The namespace
std is deliberately short so that using the prefix is not that inconvenient. Prefer to prefex items in the standard namespace with std::`.

Variable Names

{              vector    nameTemp          { };
                vector  > nameContainer     { };
                                  bool    Continue        ( true );
                                  char    c                 { };


That just looks weird.

vector             nameTemp          { };
 vector  > nameContainer     { };
 bool                      Continue          ( true ); // Though careful `continue` is a reserved word.
 char                      c                 { };


Looks more normal. But more importantly. You should declare variables as close to the point of use as you can. Don't use the C technique of declaring everything at the top of the function. Declare it just before you use it.

This is long winded way

while ( ( c = cin.get ( ) ) != '\n' )
    {
        nameTemp.push_back ( c );
        Continue = false;
    }


of saying

std::getline(std::cin, nameTemp);


Sure you can do this.

/*  Throw away temp     */
    nameTemp.erase ( nameTemp.begin ( ), nameTemp.end ( ) );


or you can just call clear()

nameTemp.clear();


Or you can just declare it at the correct location so that the constructor initializes it correctly before use.

std::string line;
    while(std::getline(std::cin, line))
    {        
         // Though this also pushes empty line (we can work around that)
         nameContainer.push_back(line);
    }


Sort every iteration not required

You sort after every iteration.

/*  Comparison Lambda   */
    /*  Descending order; substitute return ()    */
    sort (  nameContainer.begin ( ), 
            nameContainer.end ( ), 
            [ ] ( const vector  & a, const vector  & b )
                { return a < b; } );


You could just sort once after you have loaded all the words.

This looks it can be done with a tab:

if ( RowNumber  9 )    { cout << "   "; }


I would just do:

cout << "\t";


You are doing a binary search of a word

You should be doing a binary search of the container.

for ( const auto & Column : nameContainer )
        {   /*                      Binary search through 0 index   */ 
                bool Result     =   binary_search (  Column.begin ( ),
                                                     Column.end ( ),
                                                     nameTemp.at ( 0 )  );


This is what I would do:

int main()
{
    // Read a line into a string (stopping at the '\n')
    std::string line;
    std::getline(std::cin, line);

    // Treat the line we just read as a stream.
    // That allows us to read words from the line very easily.
    std::stringstream lineStream(line);

    std::vector   words;
    std::string                word;
    // Read a set of space separated words from the stream 
    while(lineStream >> word)
    {
        words.push_back(lines);
    }

    /* An alternative to the loop above is:
    std::vector    words(std::istream_iterator(lineStream),
                                      std::istream_iterator());
    */

    // Sort the array.
    std::sort(std::begin(words), std::end(words)); // default order is fine.

    // Print the array
    std::copy(std::begin(words), std::end(words),
              std::ostream_iterator(std::cout, "\n"));

    std::string   find;
    while(std::cin >> find)
    {
        auto findPos = std::lower_bound(std::begin(words), std::end(words),find);
        if (findPos == std::end(words)) {
            std::cout << find << " is missing\n";
        }
        else {
            std::cout << find << " at " << std::distance(std::begin(words), findPos) << "\n";
        }
    }
}

Code Snippets

#include < iostream >
#include < string > 
#include < algorithm >
#include < vector >
using namespace std;
{              vector < int >   nameTemp          { };
                vector < vector < int > > nameContainer     { };
                                  bool    Continue        ( true );
                                  char    c                 { };
vector < int >            nameTemp          { };
 vector < vector < int > > nameContainer     { };
 bool                      Continue          ( true ); // Though careful `continue` is a reserved word.
 char                      c                 { };
while ( ( c = cin.get ( ) ) != '\n' )
    {
        nameTemp.push_back ( c );
        Continue = false;
    }

Context

StackExchange Code Review Q#87507, answer score: 8

Revisions (0)

No revisions yet.