3
\$\begingroup\$

I have file called Network.txt which contains SSID and BSSID as below:

  SSID                              BSSID               MODE             FREQ       RATE       SIGNAL   SECURITY   ACTIVE
'Bards Tale'                  30:DA:9B:70:EE:F4   Infrastructure   2412  MHz   54 MB/s    47       WPA2              no
'Alaskan Pale'                70:37:73:B7:3A:53   Infrastructure   2412 MHz   54 MB/s    44       WPA2             yes
'Abita Turbodog'              9C:1E:04:4C:59:FC   Infrastructure   2412 MHz   54 MB/s    7        WPA WPA2         no
'Alaskan Amber'               12:62:EB:2C:C0:06   Infrastructure   2422 MHz   54 MB/s    2        WPA WPA2         no

I want to get SSID and BSSID in safe and efficient way. Here is my approach, which is working fine; can it be improved?

void GetSSIDAndBSSID(const std::string &temp)
    {
        if (temp.find("'") != std::string::npos)
        {

            std::string::size_type end_pos = temp.find_last_of(":");
            if (end_pos == std::string::npos)
                end_pos = temp.size();

     std::string FirstWord = temp.substr(1, end_pos+2); 
    std::cout <<"SSID="<< FirstWord.substr(0,temp.find_last_of("'")-1) << std::endl;
   std::cout<<"BSSID=" << FirstWord.substr(temp.find_first_of(":")-2,temp.find_last_of(":")+2) << std::endl;
        }
    }


int main()
{
        ifstream fin("pathTO/Network.txt");
        std::string line;
        while(getline(fin,line))
        {

                GetSSIDAndBSSID(line);

        }                                      
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

Everything @Edward said

Additionally I would look at your style. The standard pattern for reading a single item is:

stream >> X

The standard pattern for reading all the items from the stream is:

while (stream >> X)
{
    // action with X like printing it out
    // Separation of concerns as Edward noted.
}

To achieve this pattern is relatively trivial:

  1. Define a class to hold the data from one line.
  2. Define an input operator for the class.

    class SSIDAndBSSID
    {
        // Add members for each field.
        // Have a default constructor.
    
        friend std::istream& operator>>(std::istream& str, SSIDAndBSSID& data)
        {
            std::string line;
            if (std::getline(stream, line)) {
                // Break stream into parts and place into `data`
                // This is a friend function and thus has full access
                // to all members.
                //
                // Note: If there is an error in processing,
                //       then you need to set the bad bit on the stream.
                //       This will force the loop to exit (by indicating
                //        the read failed).
                //         stream. setstate(std::ios::fail);
    
            }
            return stream; 
        }
    };
    

Once you have that done, then not only can you loop over the file, but you can simply read it into a container with:

std::ifstream  file("Network.txt");
std::vector<SSIDAndBSSID>   facts(std::istream_iterator<SSIDAndBSSID>(file),
                                  std::istream_iterator<SSIDAndBSSID>());

Additionally I would use the stream operators to read your input data. Your current code is hard to read.

std::string  line;
if (std::getline(stream, line))
{
    // Got a line
    std::stringstream lineStream(line);

    char x;
    std::string ssid;
    std::string bssid;

    if (lineStream >> x && x == '\'' && std::getline(lineStream, ssid, '\'') && lineStream >> bssid)
    {
        // We enter here iff:
        //   The first non-whitespace character is '
        //   We got a word that was terminated by    '
        //   We managed to read a word (dropping prefix white space)
    }
    else
    {
        stream. setstate(std::ios::fail);
    }
}
\$\endgroup\$
4
\$\begingroup\$

I see a number of things that may help you improve your code.

Use the required #includes

The code uses ifstream which means that it should #include <fstream>. It was not difficult to infer, but it helps reviewers if the code is complete. In this case, the code needs the following:

#include <string>
#include <iostream>
#include <fstream>

Separate concerns

Instead of having the single routine extract field and print them, I'd recommend separating those into separate functions. It makes the code clearer and allows flexibility if, for example, you wanted to sort the entries before printing them.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. I don't know that you've actually done that, but it seems so because the code within main omits the namespace in several places.

Fix the bug

Right now, the code reports the BSSID without the first character. That's a bug caused because the first "-2" in the last line of GetSSIDAndBSSID should actually be -3. Also, as pasted, your main is missing a closing }.

Be sure of your inputs

We are left to infer, from the sample and from your code, the format of the file that is to be parsed. Perhaps you had to figure it out from samples yourself. Either way, it's very helpful to document, as comments, just exactly what the code is expecting so that if any of those assumptions ever need to change, it will be clear what needs to be changed. For example, it appears that there is an implicit assumption that the last ":" on the line is used to figure out the substring length that includes the two fields of interest. However, it appears to me that the fields are of fixed width, which would make it very efficient to simply used a fixed index into the string. Also, your code seems to assume but doesn't test that the first character on the line is "'".

Use consistent formatting

The code as posted has inconsistent indentation which makes it hard to read and understand. Pick a style and apply it consistently.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.