Extracting file names from text file

Question

I need to extract file names with their extension from an input text file to a string vector. The input text file is quite messy and serves as configuration file for some application.

What I know about the file names I am trying to extract is they are preceded by a file = mention, the filename is quoted between ' ' or " ".

Example : file="name.abc". I also have no guarantee of how spacing is : It may be file="name.abc", file = "name.abc" , file= "name.abc"... And extension can be of different lengths.

So I tried the following code:

std::vector<std::string> attachment_names;
std::istringstream words(text_content);
std::string word;
std::string abc_extension(".abc"); // My code should support any extension
while (words >> word)
{
    auto extension_found = word.find(abc_extension);
    if (extension_found != word.npos)
    {
        auto name_start = word.find("'") + 1; 
             //I am not even sure the file is quoted by ''

        std::string attachment_name = word.substr(name_start, (extension_found + 3) - name_start + 1); 
             //Doing this annoys me a bit... Especially that the extension may be longer than 3 characters

        attachment_names.push_back(attachment_name);
    }
}

Is there a nicer way of doing this? Is there a possibility to rely more on the file caption to support any extension?

Answer 1

Here are a number of things that may help you improve your code.

Use the required #includes

The code uses std::vector which means that it should #include <vector>. It was not difficult to infer, but it helps reviewers if the code is complete. This code appears to require the following:

#include <vector>
#include <sstream>
#include <string>
#include <iostream>

Provide complete code to reviewers

This is not so much a change to the code as a change in how you present it to other people. Without the full context of the code and an example of how to use it, it takes more effort for other people to understand your code. This affects not only code reviews, but also maintenance of the code in the future, by you or by others. One good way to address that is by the use of comments. Another good technique is to include test code showing how your code is intended to be used and the expected results. The code, as posted, is just a program fragment, so I turned it into a function for testing:

std::vector<std::string> fetch_names(const std::string &text_content, const std::string &ext) {
    std::vector<std::string> attachment_names;
    std::istringstream words(text_content);
    std::string word;
    while (words >> word) {
        auto extension_found = word.find(ext);
        if (extension_found != word.npos) {
            auto name_start = word.find("'") + 1; 
            std::string attachment_name = word.substr(name_start, (extension_found + 3) - name_start + 1); 
            attachment_names.push_back(attachment_name);
        }
    }
    return attachment_names;
}

Fix the bugs

As posted, the extension was named pst_extension but was used as abc_extension so it wouldn't compile without some alteration. More serious, however, is that the matching appears to be flawed. It purports to look for a filename surrounded by single quotes but that's not what it actually does. I wrote a small test program:

int main(int argc, char *argv[]) {
    if (argc != 2) {
        std::cout << "Usage: getfile filename\n";
        return 0;
    }
    std::ifstream in{argv[1]};
    for (std::string fn; std::getline(in, fn); ) {
        auto names = fetch_names(fn, ".abc");
        unsigned i=0;
        for (const auto &file : names) {
            std::cout << ++i << "\t[" << file << "]\n";
        }
    }
}

I then used it with this file:

file = "one.abc"
file='two.abc"
file   == three.abcd
file=four.ab
file='five.abcd'  file2='six.abcd'
file='seven.abc' file='eight.abc'

I would expect the output to be this:

1   [seven.abc]
2   [eight.abc]

But what it actually reported was this:

1   ["one.abc]
1   [two.abc]
1   [three.abc]
1   [five.abc]
2   [six.abc]
1   [seven.abc]
2   [eight.abc]

Consider regular expressions

First, it appears that the "file=" portion of the line is ignored. The code actually acts on any matchine string within single quotes. Assuming that's intentional, we can use regular expressions very easily for this:

#include <regex>
#include <iterator>

std::vector<std::string> fetch_names(const std::string &text_content, 
            const std::string &ext) {
    std::vector<std::string> file;
    std::regex file_regex{"'(\\S+"+ext+")'"};
    auto begin = std::sregex_iterator{text_content.begin(),
              text_content.end(), file_regex};
    auto end = std::sregex_iterator{};
    for (auto it = begin; it != end; ++it) {
        file.push_back((*it)[1]);
    }
    return file;
}

This works with a fairly simple regular expression that looks for opening and closing single quote characters and assumes the filename is made of non-whitespace characters only. If that's not the case for your program (e.g. if you wish to accept spaces within the file name), then you can adjust the regex accordingly. For example, if you wish to allow either single or double quotes (but they must match) and the filename can include alphanumeric plus spaces and underscores, we can use this:

std::regex file_regex{"('|\")([ _[:alnum:]]+"+ext+")\\1"};

In this case, the actual filename portion is in the second grouping, so we would account for that by changing the loop:

for (auto it = begin; it != end; ++it) {
    file.push_back((*it)[2]);
}

If you want the code to explicitly look for the "file = " portion of the line as well, it's as simple as changing the regex to this:

std::regex file_regex{"file\\s*=\\s*('|\")([ _[:alnum:]]+"+ext+")\\1"};

Note that using a regex is often slower than alternatives, such as writing your own state machine, but is often fast enough and is extremely flexible. Note for example, that we actually construct the regex at runtime from a passed parameter. This is both very flexible and potentially dangerous, so any user input should be sanitized before use.