2
\$\begingroup\$

I got a mixture of two languages ​​C and C ++

The code finds the folder my documents and creates additional folders in it when the program starts, I found no other way to implement this better. I also did not find any ready-made example where all this has already been implemented so as not to make a bicycle, I like when the code looks compact, but it looks bad for me.

template <typename T1, typename T2>
void knownFolder(T1& setKnownFolderID, T2& getDisplayName)
{
    PWSTR pszFolderPath;

    for (;;) // !!!>
    {
        if (FAILED(SHGetKnownFolderPath(setKnownFolderID, 0, nullptr, &pszFolderPath)))
            continue;


        getDisplayName = pszFolderPath;
        CoTaskMemFree(pszFolderPath);

        break;  // <!!
    }
}

/

template <typename T2>
void folderLink(T2& setDisplayName, T2& getDisplayName)
{
    getDisplayName = setDisplayName + L"\\" + getDisplayName;

    for (;;) // !!!>
    {
        if (!filesystem::exists(getDisplayName))
            if (!filesystem::create_directory(getDisplayName))
                continue;


        break; // <!!
    }
}

/

wstring folder_Documents,                    // C:\ ... \Documents
        folder_Microsoft = L"Microsoft",     // C:\ ... \Documents\Microsoft

        folder_XP = L"XP",
        folder_XP_Alpha = L"Alpha",
        folder_XP_Beta = L"Beta",

        folder_Win7 = L"Win7",
        folder_Win7_Alpha = L"Alpha",
        folder_Win7_Beta = L"Beta";

/

void foldersystem()
{
    knownFolder(FOLDERID_Documents, folder_Documents); // Documents

    folderLink(folder_Documents, 
        folder_Microsoft); // Microsoft

        folderLink(folder_Microsoft,
            folder_XP); // XP

            folderLink(folder_XP, folder_XP_Alpha);
            folderLink(folder_XP, folder_XP_Beta);

        folderLink(folder_Microsoft,
            folder_Win7); // Win7

            folderLink(folder_Win7, folder_Win7_Alpha);
            folderLink(folder_Win7, folder_Win7_Beta);

}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Use std::filesystem::path for paths

C++17 gives you a canonical type to represent filesystem paths, so use that. This avoids having to create templates of all your functions. It also allows you to combine paths together in a portable way:

void folderLink(std::filesystem::path &setDisplayName, std::filesystem::path &getDisplayName)
{
    getDisplayName = setDisplayName / getDisplayName;
    ...
}

Use std::filesytem::create_directories()

This function will create the desired directory plus all its parent directories if they do not yet exist. Also, it will not report an error if the desired directory already exists. So you could write:

void foldersystem()
{
    std::filesystem::path folder_Documents = ...; // get known folder as a path here

    std::filesystem::create_directories(folder_Documents / "Microsoft" / "XP" / "Alpha");
    std::filesystem::create_directories(folder_Documents / "Microsoft" / "XP" / "Beta");
    std::filesystem::create_directories(folder_Documents / "Microsoft" / "Win7" / "Alpha");
    std::filesystem::create_directories(folder_Documents / "Microsoft" / "Win7" / "Beta");
}

Don't infinitely retry on errors

You are using a very weird pattern with infinite for-loops:

for(;;) {
    if (something_fails())
        continue;

    break;
}

If the thing you are trying to do fails, you are retrying it indefinitely. But if it fails once, why do you expect it will work the next time? The right thing to do at this point is just to return an error.

Prefer returning results instead of using output parameters

If a function returns something, return that instead of passing a reference to an output parameter. Combined with the use of std::filesystem::path, knownFolder() then becomes:

std::filesystem::path knownFolder(KNOWNFOLDERID fodlerID)
{
    PWSTR pszFolderPath;

    if (FAILED(SHGetKnownFolderPath(folderID, 0, nullptr, &pszFolderPath)))
        throw std::runtime_error("Could not get known folder");

    std::filesystem::path result = pszFolderPath;
    CoTaskMemFree(pszFolderPath);
    return result;
}

Use an array of paths if you have many of them

If you have many paths to create, then store them in an array, and use a for-loop to create them all. Here is an example:

static const std::filesystem::path required_subdirectories[] = {
    "XP/Alpha",
    "XP/Beta",
    "Win7/Alpha",
    "Win7/Beta",
};

void foldersystem()
{
    std::filesystem::path folder_Documents = ...; // get known folder as a path here

    for (auto &subdirectory: subdirectories) {
            std::filesystem::create_directories(folder_Documents / "Microsoft" / subdirectory);
    }
}

Note that using the forward slash is perfectly fine here; the std::filesystem functions understand this as a directory separator.

\$\endgroup\$
5
  • \$\begingroup\$ thanks, I’m free to try to implement, but so far I have not understood how to keep the data in variables or in arrays \$\endgroup\$
    – 1221
    Commented Oct 22, 2019 at 18:46
  • \$\begingroup\$ @1221 So you want to know how to store the paths in an array, and use a loop to create those paths? \$\endgroup\$
    – G. Sliepen
    Commented Oct 22, 2019 at 20:03
  • \$\begingroup\$ I am interested in how to access these paths, in my example I created a variable for each path. \$\endgroup\$
    – 1221
    Commented Oct 23, 2019 at 3:32
  • \$\begingroup\$ I'm not sure what you mean by accessing. But maybe you can create a question on stackoverflow.com for this? \$\endgroup\$
    – G. Sliepen
    Commented Oct 23, 2019 at 5:48
  • \$\begingroup\$ You may want to add to the response the user of std::filesystem::permissions in order to avoid errors on the execution of the program in case the user can not create the directory \$\endgroup\$
    – camp0
    Commented Oct 23, 2019 at 7:48

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.