3
\$\begingroup\$

What

I have this program that allows me assigning tags for directories and then change working directory to the directories via their tags.

Installing

In your home directory, create a folder .dt. In it, put dt.bat, dt_engine.exe and the tag file table. What comes to table, my looks like this:

home C:\Users\Rodion Efremov
prev C:\Users\Rodion Efremov\Documents\NetBeansProjects\WindowsRegistryLayer 
wrl C:\Users\Rodion Efremov\Documents\NetBeansProjects\WindowsRegistryLayer

Finally, you may prefer to make a doskey for dt.bat so that you may type only dt TAG-

Usage

Just type dt TAG in order to switch to the directory with tag TAG. Use only dt in order to jump between the current and previous directories. Finally, note that tags are matched via Levenshtein-distance, so that you don't need to type the tag always the right way.

Code

dtengine.cpp

#include <algorithm>
#include <cctype>
#include <fstream>
#include <functional>
#include <iostream>
#include <limits>
#include <locale>
#include <sstream>
#include <stdlib.h>
#include <string>
#include <map>
#include <windows.h>

#define MAX_ENVIRONMENT_VARIABLE_NAME_LENGTH 128
#define MAX_TABLE_FILE_LINE_LENGTH 512
#define DT_TABLE_FILE_NAME "\\.dt\\table"

static std::string get_home_directory_name() {
    char* buffer = (char*) calloc(
        MAX_ENVIRONMENT_VARIABLE_NAME_LENGTH,
        sizeof(char));

    if (!buffer) {
        return NULL;
    }

    size_t requiredSize;

    int ret = getenv_s(
        &requiredSize,
        buffer, 
        MAX_ENVIRONMENT_VARIABLE_NAME_LENGTH, 
        "HOMEPATH");

    return std::string(buffer);
}

static std::string get_table_file_name() {
    std::string user_dir = get_home_directory_name();
    user_dir += DT_TABLE_FILE_NAME;
    return user_dir;
}

static std::map<std::string, std::string> get_table(
    std::ifstream& file
) {
    std::map<std::string, std::string> table;
    char buffer[MAX_TABLE_FILE_LINE_LENGTH];

    while (!file.eof()) {
        file.getline(buffer, MAX_TABLE_FILE_LINE_LENGTH);
        std::string line = buffer;
        std::istringstream iss(line);
        std::string tag;
        std::string path;

        iss >> tag;
        iss.getline(buffer, MAX_TABLE_FILE_LINE_LENGTH - 1 - tag.length());
        path = buffer;
        table[tag] = path;
    }

    return std::move(table);
}

static inline std::string &ltrim(std::string &s) {
    s.erase(s.begin(), std::find_if(s.begin(), s.end(),
        std::not1(std::ptr_fun<int, int>(std::isspace))));
    return s;
}

static inline std::string &rtrim(std::string &s) {
    s.erase(std::find_if(s.rbegin(), s.rend(),
        std::not1(std::ptr_fun<int, int>(std::isspace))).base(), s.end());
    return s;
}

static inline std::string &trim(std::string &s) {
    return ltrim(rtrim(s));
}

static size_t levenshtein_distance(
    std::string const& str1,
    std::string const& str2,
    size_t i1,
    size_t i2) {
    if (i1 == 0) {
        if (i2 == 0) {
            return 0;
        } else {
            return i2;
        }
    } else if (i2 == 0) {
        return i1;
    }

    return min(
        min(levenshtein_distance(str1, str2, i1 - 1, i2) + 1,
            levenshtein_distance(str1, str2, i1, i2 - 1) + 1),
        levenshtein_distance(str1, str2, i1 - 1, i2 - 1) +
                            (str1[i1 - 1] != str2[i2 - 1] ? 1 : 0));
}

static size_t levenshtein_distance(
    std::string const& str1,
    std::string const& str2) {
    return levenshtein_distance(
        str1,
        str2,
        str1.length(),
        str2.length());
}

static void string_to_lowercase(std::string& str) {
    std::transform(str.begin(), str.end(), str.begin(), ::tolower);
}

static std::string get_current_path() {
    char* buffer = (char*)calloc(512, sizeof(char));
    GetCurrentDirectoryA(512, buffer);
    std::string path = buffer;
    return std::move(path);
}

static int go_to_previous_directory() {
    std::string table_file_name = get_table_file_name();
    std::ifstream file(table_file_name.c_str(), std::ifstream::in);
    std::map<std::string, std::string> table = get_table(file);
    std::string prev_tag = "prev";
    std::string next_directory;
    file.close();

    for (std::pair<const std::string, std::string>& p : table) {
        std::string tag = p.first;
        std::string lowercase_tag = tag;
        string_to_lowercase(lowercase_tag);

        if (lowercase_tag == prev_tag) {
            next_directory = trim(p.second);
            std::ofstream file_out(
                table_file_name.c_str(), 
                std::ofstream::out | std::ofstream::trunc);

            // Map tag 'prev' to 'current_path':
            std::string current_path = get_current_path();
            table[prev_tag] = current_path;

            char* separator = "";
            for (std::pair<const std::string, std::string>& pp : table) {
                std::string tag = pp.first;
                std::string path = pp.second;
                trim(tag);
                trim(path);
                file_out << separator << tag.c_str() << " " << path.c_str();
                separator = "\n";
            }

            file_out.close();
            std::cout << "CD \"" << next_directory << "\"" << std::endl;  
            return 0;
        }
    }

    // The tag 'prev' not in the table, create it:
    table[prev_tag] = get_current_path();
    std::ofstream file_out(
        table_file_name.c_str(),
        std::ofstream::out | std::ofstream::trunc);

    char* separator = "";

    for (std::pair<const std::string, std::string>& p : table) {
        file_out << separator << p.first.c_str() << " " << p.second.c_str();
        separator = "\n";
    }

    file_out.close();
    return 0;
}

int main(int argc, char* argv[])
{
    if (argc == 1) {
        return go_to_previous_directory();
    }

    std::string target_tag = argv[1];
    std::string table_file_name = get_table_file_name();
    std::ifstream file(table_file_name.c_str(), std::ifstream::in);
    std::map<std::string, std::string> table = get_table(file);

    if (table.empty()) {
        file.close();
        return 2;
    }

    string_to_lowercase(target_tag);     
    file.close(); 

    size_t best_levenshtein_distance = MAX_TABLE_FILE_LINE_LENGTH;
    std::string best_tag;
    std::string path;

    for (std::pair<const std::string, std::string>& p : table) {
        std::string tag = p.first;
        string_to_lowercase(tag);
        size_t distance = levenshtein_distance(tag, target_tag);

        if (best_levenshtein_distance > distance) {
            best_levenshtein_distance = distance;
            best_tag = tag;
            path = p.second;
        }
    }

    std::string current_path = get_current_path();
    table["prev"] = current_path;

    // Save the 'table["prev"]':
    std::ofstream file_out(
        table_file_name.c_str(),
        std::ofstream::out | std::ofstream::trunc);

    char* separator = "";

    for (std::pair<const std::string, std::string>& p : table) {
        file_out << separator << p.first.c_str() << " " << p.second.c_str();
        separator = "\n";
    }

    file_out.close();
    path = trim(path);

    if (path.empty()) {
        return 1;
    } else {
        std::cout << "CD \"" << path << "\"" << std::endl;
        return 0;
    }  
}

dt.bat

@echo off
del "%HOMEPATH%\.dt\cd.bat"

if [%1] == [] (
  dt_engine.exe > "%HOMEPATH%\.dt\cd.bat"
) else (
  dt_engine.exe %1 > "%HOMEPATH%\.dt\cd.bat"
)

if %ERRORLEVEL% NEQ 0 (
  echo Something went wrong. 
) else (
  "%HOMEPATH%\.dt\cd.bat"
)

del "%HOMEPATH%\.dt\cd.bat"

Critique request

My code seems like a procedural mess. How can I improve it?

\$\endgroup\$
5
  • 1
    \$\begingroup\$ Can you use C++17? I believe VC++ implemented it already, it is enabled by flag /std:c++latest as described by this post. \$\endgroup\$ Sep 5 '17 at 13:32
  • 1
    \$\begingroup\$ I fixed the weird formatting of the quoted block. It seems like each block formatting specifier adds a "scope". To apply inner formatting, just indent 4 more spaces for code. \$\endgroup\$ Sep 5 '17 at 13:38
  • 1
    \$\begingroup\$ looks like a memleak in get_home_directory_name, with the buffer you allocate. IMHO it is better to use std:vector, it behaves just as your buffer but without the memleak. \$\endgroup\$
    – AndersK
    Sep 5 '17 at 17:02
  • \$\begingroup\$ @AndersK. You are right. I never released the chars. \$\endgroup\$
    – coderodde
    Sep 5 '17 at 17:12
  • 1
    \$\begingroup\$ @AndersK. Comments are for seeking clarification to the question, and may be deleted. Please put all bug reports and suggestions for improvements in answers, even if the answer is short. \$\endgroup\$ Sep 5 '17 at 18:18
1
\$\begingroup\$
#include <algorithm>
#include <cctype>
#include <fstream>
#include <functional>
#include <iostream>
#include <limits>
#include <locale>
#include <sstream>
#include <stdlib.h>
#include <string>
#include <map>
#include <windows.h>

When I saw this I immediately got the gist of what is going on... But the party didn't stop there:

#define MAX_ENVIRONMENT_VARIABLE_NAME_LENGTH 128
#define MAX_TABLE_FILE_LINE_LENGTH 512
#define DT_TABLE_FILE_NAME "\\.dt\\table"

When scanning through the code I usually check for missing/redundant includes. The amount of included files in this case was enough to overwhelm the ability. Although splitting into multiple files might make compilation slower, it would vastly improve coherence over what is being dealt with. Included files also serve as "titles" for fast readers: it is easier to predict what code is going to be about if includes are chosen wisely.

The macros could be changed to constexpr variables, even the string literal: constexpr const char* dt_table_filename = "...". Inlining of the variables wouldn't buy a lot anyway.


Abstraction

The first thing that comes to mind is of course table class that would encapsulate creation, management, access to table. One can go berserk with the parsing phase, to make it super obvious what have actually gone wrong. Ideally table shouldn't change after creation, and only one instance should be present at any given time.


There are other things that might cause trouble in the code. Especially all usages of C style string manipulation functions are undefined as written. Might not be a problem on x86.

Usage of stuff that is deprecated or even removed in C++17. There are lots of occurences, namely std::ptr_fun, std::not1, etc.

Lots of C style functions.

for (std::pair<const std::string, std::string>& p : table)

Why explicit type?

std::ifstream file(table_file_name.c_str(), std::ifstream::in);
std::map<std::string, std::string> table = get_table(file);
std::string prev_tag = "prev";
std::string next_directory;
file.close();

This could be put into get_table(), delegating the table_file_name.

int go_to_previous_directory()

My gut feeling tells me there should be std::stack somewhere, not sure though.

Lots of statics. I don't think that it will buy anything in this case.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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