3
\$\begingroup\$

It is a Windows command line utility for counting lines of code in a source code directory. It will count loc and sloc. I read from a file character by character using fgetc in the CountLines(...) function. I set a buffer size of 4096 bytes. Observed speed is not much different from the default case. How can I improve it. Sample usage:

D:\proj\ext foss\Quake-III-Arena-master>clocw c cpp h

Physical Lines Of Code (loc): 472284

Logical Lines Of Code (sloc): 262573

Processing time: 1.286 Seconds(1286 Milliseconds)

Here is the source code:( It is a github project: https://github.com/mmj-the-fighter/clocw )

 /*
Attribution:
https://stackoverflow.com/users/13566062/song-zhu has written major parts of ProcessFiles function.
Stackoverflow Post : https://stackoverflow.com/questions/63386581/how-can-you-iterate-though-directories-without-using-any-recursive-functions-in
*/
#include <Windows.h>
#include <tchar.h>
#include <iostream>
#include <string>
#include <queue>
#include <vector>
#include <unordered_set>
#include <chrono>
#include <stdio.h>



class QuickProfiler
{
public:
    std::chrono::system_clock::time_point t0;
    long long millis;
    double seconds;
    long long diff;

    inline void Start()
    {
        t0 = std::chrono::high_resolution_clock::now();
    }

    inline void Stop()
    {
        using namespace std::chrono;
        system_clock::time_point t1 = high_resolution_clock::now();
        millis = duration_cast<milliseconds>(t1 - t0).count();
        diff = (t1 - t0).count();
        seconds = (double)millis / 1000.0;
    }
};

int ReadExtensionFromPath(TCHAR* path, TCHAR* extension)
{
    TCHAR c;
    TCHAR ext[MAX_PATH];
    int i = 0;
    int j = 0;

    //find the length
    while (*(path + i) != '\0') {
        ++i;
    }

    if (i == 0) {
        return -1;
    }


    //read from last char till a dot
    --i;
    while (i >= 0) {
        c = *(path + i);
        if (c != '.') {
            ext[j++] = c;
        }
        else{
            break;
        }
        --i;
    }
    ext[j] = '\0';

    // reverse buffer
    i = 0;
    j = j - 1;
    while (i < j)
    {
        c = ext[i];
        ext[i] = ext[j];
        ext[j] = c;
        ++i;
        --j;
    }

    //copy ext
    i = 0;
    while ((c = *(ext + i)) != '\0')
    {
        *(extension + i) = c;
        ++i;
    }
    *(extension + i) = '\0';

    return 0;
}

bool IsDesiredFileType(TCHAR* filename, std::unordered_set<std::wstring> sourcefile_extensions)
{
    TCHAR extension[MAX_PATH];
    ReadExtensionFromPath(filename, extension);
    std::wstring ext(extension);
    
    if (sourcefile_extensions.find(ext) != sourcefile_extensions.end())
        return true;
    else
        return false;
}

//For analyzing C, C++, Java, C# code comments and source poritions
void CountLines(const wchar_t *filename, unsigned int* pploc, unsigned int *psloc)
{
    unsigned int loc = 0;
    unsigned int sloc = 0;
    int ch, prevChar = 0;
    int foundCode = 0;
    int foundSLComment = 0;
    int foundMLComment = 0;
    int fwslashcount = 0;

#define BUFFERSIZE_CLOCW 4096
    static char buffer[BUFFERSIZE_CLOCW];
    FILE* f;
    _wfopen_s(&f, filename, L"r");
    if (f == NULL) {
        std::wcout << "Cannot Open : " << filename << std::endl;
        return;
    }
        
    if (setvbuf(f, buffer, _IOFBF, BUFFERSIZE_CLOCW) != 0)
    {
        std::cout << "setvbuf failed" << std::endl;
        exit(EXIT_FAILURE);
    }

    while ((ch = fgetc(f)) != EOF)
    {
        //std::cout << (char)ch;
        if (foundSLComment == 0 && foundMLComment == 0 && foundCode == 0){
            foundCode = isalnum(ch);
        }
        if (foundMLComment == 0){
            if (ch == '/')
            {
                if (prevChar == '/'){
                    foundSLComment = 1;
                }
            }
            else if (ch == '*'){
                if (prevChar == '/' && foundSLComment == 0){
                    foundMLComment = 1;
                }
            }
        }
        else {
            if (ch == '/' && prevChar == '*'){
                foundMLComment = 0;
            }
        }

        if (ch == '\n'){
            ++loc;
            if (foundCode) {
                ++sloc;
            }
            foundCode = 0;
            foundSLComment = 0;
        }
        prevChar = ch;
    }
    ++loc;
    if (foundCode) {
        ++sloc;
    }
    fclose(f);
    *pploc += loc;
    *psloc += sloc;
#undef BUFFERSIZE_CLOCW
}

void ProcessFiles(std::wstring sourcepath, bool isRecursive, std::unordered_set<std::wstring> sourcefile_extensions, unsigned int* pphysicalLinecount, unsigned int *psourceLinecount)
{
    bool isAllFilesToBeProcessed = false;
    std::wstring allfilesext = L"*";
    if (sourcefile_extensions.find(allfilesext) == sourcefile_extensions.end())
        isAllFilesToBeProcessed = false;
    else
        isAllFilesToBeProcessed = true;

    std::queue<std::wstring> qFolders;
    qFolders.push(sourcepath);

    WIN32_FIND_DATA findResult;
    HANDLE handle = NULL;

    while (qFolders.size() > 0)
    {
        std::wstring path = qFolders.front();
        path.append(_T("\\*"));
        handle = FindFirstFile(path.c_str(), &findResult);
        do
        {
            if (findResult.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
            {
                if (lstrcmp(_T("."), findResult.cFileName) == 0 || lstrcmp(_T(".."), findResult.cFileName) == 0)
                {
                    continue;
                }
                if (isRecursive)
                {
                    path = qFolders.front();
                    path.append(_T("\\")).append(findResult.cFileName);
                    qFolders.push(path);
                }
            }
            else {
                //wcout << findResult.cFileName << endl;
                if (isAllFilesToBeProcessed || 
                    IsDesiredFileType(findResult.cFileName, sourcefile_extensions))
                {
                    path = qFolders.front();
                    path.append(_T("\\")).append(findResult.cFileName);
                    //wcout << path << endl;
                    CountLines(path.c_str(), pphysicalLinecount, psourceLinecount);
                    //CountLinesSkelton(path.c_str(), pphysicalLinecount, psourceLinecount);
                    //cout << linecount << endl;
                }

            }
        } while (FindNextFile(handle, &findResult));
        qFolders.pop();
    }
    if (handle)
    {
        FindClose(handle);
        handle = NULL;
    }
}

int _tmain(int argc, _TCHAR* argv[])
{
    unsigned int physicalLinecount = 0;
    unsigned int sourceLinecount = 0;
    QuickProfiler profiler;
    std::unordered_set<std::wstring> sourcefile_extensions;
    TCHAR currentDirectory[MAX_PATH];
    DWORD result;
    profiler.Start();
    result = GetCurrentDirectory(MAX_PATH, currentDirectory);
    if (result == 0)
    {
        return 1;
    }

    bool isRecursive = true;
    std::wstring nonRecursiveOption = L"-nr";
    std::wstring recursiveOption = L"-r";
    if (argc == 1)
    {
        sourcefile_extensions.insert(_T("cpp"));
        sourcefile_extensions.insert(_T("h"));
        sourcefile_extensions.insert(_T("hpp"));
    }
    else if (argc == 2)
    {
        std::wstring option = std::wstring(argv[1]);
        if (option == nonRecursiveOption){
            isRecursive = false;
            sourcefile_extensions.insert(_T("cpp"));
            sourcefile_extensions.insert(_T("h"));
            sourcefile_extensions.insert(_T("hpp"));
        }
        else if (option == recursiveOption){
            isRecursive = true;
            sourcefile_extensions.insert(_T("cpp"));
            sourcefile_extensions.insert(_T("h"));
            sourcefile_extensions.insert(_T("hpp"));
        }
        else{
            sourcefile_extensions.insert(option);
        }
    }
    else
    {
        std::wstring option = std::wstring(argv[1]);
        if (option == nonRecursiveOption){
            isRecursive = false;
        }
        else if (option == recursiveOption){
            isRecursive = true;
        }
        else{
            sourcefile_extensions.insert(option);
        }

        for (int i = 2; i < argc; i++)
        {
            std::wstring option = std::wstring(argv[i]);
            sourcefile_extensions.insert(option);
        }
    }
     
    //ProcessFiles(_T("D:\\sample1"), false, sourcefile_extensions, &physicalLinecount, &sourceLinecount);
    ProcessFiles(currentDirectory, isRecursive, sourcefile_extensions, &physicalLinecount, &sourceLinecount);
    profiler.Stop();
    std::cout << "Physical Lines Of Code (loc): " << physicalLinecount << std::endl;
    std::cout << "Logical Lines Of Code (sloc): " << sourceLinecount << std::endl;
    std::cout << "Processing time: " << profiler.seconds << " Seconds"
        << "(" << profiler.millis << " Milliseconds)" << std::endl;
    return 0;
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ I think your comment detection logic is flawed, since // and /* may also be embedded in string literals and hence not indicate comments at all. \$\endgroup\$ Commented Oct 26, 2022 at 18:14
  • \$\begingroup\$ yeah this code will fail on nested comments. In general, detecting comments correctly is not as simple as it may seem at first, it basically requires you to parse the file for a very simple grammar \$\endgroup\$ Commented Oct 26, 2022 at 22:48

2 Answers 2

Reset to default
4
\$\begingroup\$

Try to write portable C++ code

You are using lots of Windows and MSVC-specific functions, which make your code non-portable. You are also using C functions when you could be using standard C++ equivalents, which will result in cleaner code.

If it is possible, enable the UTF-8 code page when compiling your project in Visual Studio. This will allow you to use the regular chars and std::strings for filenames and other things that might contain Unicode characters. This will then allow you to use a normal main() function. I also wonder if using std::cout inside _tmain() is correct, moving to UTF-8 will avoid that question entirely.

Also if possible, use C++17's filesystem library, which greatly simplifies scanning directories and dealing with filenames. For example:

#include <filesystem>
...
using namespace fs = std::filesystem;

int main(int argc, char *argv[]) {
    std::unordered_set<fs::path> sourcefile_extensions = {
        ".cpp", ".h", ".hpp",
    };

    for (auto& entry: fs::recursive_directory_iterator(".")) {
        if (entry.is_directory()) {
            continue;
        }

        auto extension = entry.path().extension();

        if (source_extensions.contains(extension)) {
            // open entry.path() and count
            ...
        }
    }

    ...
}

For reading the file, use std::ifstream and std::getline().

Use '\n' instead of std::endl

Use '\n' instead of std::endl; the latter is equivalent to the former, but also forces the output to be flushed, which is usually unnecessary and might have a negative effect on performance.

Avoid repeating yourself

In main(), there are three places where you add the default extensions to sourcefile_extensions. Whenever you do the same thing multiple times, try to find some way to avoid the duplication. For example, you could create a function that initializes sourcefile_extensions with the defaults, or create a static const variable holding the default extensions, so you can write:

static const std::unordered_set<std::path> default_extensions = {
    ".cpp", ".h", ".hpp",
};
...
sourcefile_extensions = default_extensions;
\$\endgroup\$
1
  • \$\begingroup\$ I missed the std::endl. \$\endgroup\$ Commented Oct 26, 2022 at 18:02
4
\$\begingroup\$

General Observations

Nice program/project! It generally looks good.

It isn't clear which version of Visual Studio / VC++ you are using. I'm using Visual Studio 2022 Professional with VC++20 and the code doesn't quite compile. I will detail the edits I made below.

  • Generally programs should default to non-recursive, that is what wc (word count) on Linux systems does, so there should be no need for a -nr flag.
  • Utility programs should be portable if possible.
  • As of C++17 the C++ standard provides a file system interface so that C++ programs can be more portable. Prefer C++ libraries of Windows libraries. The C++ libraries will take care of system specific file system details in most cases.
  • Command line programs should prefer the more portable int main() or int main(int argc, char* argv[]) over the Microsoft specific int _tmain(int argc, _TCHAR* argv[])). If you are using MFC for a GUI you might have to use _tmain() otherwise stick to the C++ standard. This is discussed on stack overflow.
  • Keep line lengths under 80 characters when possible, in the Attribution comment one line is 100 characters long and another is 146 characters long. The URL on the 146 character line should be on it's own line and is 125 characters. Many editors or IDEs default to 132 characters or less.
  • While it is sometimes necessary to time programs, it might be better to use the profiling tools provided, on Windows systems Visual Studio does provide profiling tools, there are also Gnu tools for profiling code. Profiling tools help identify where the bottlenecks in the code are, not only do they tell you how much time you spend in a single function, they also tell you how many times the function is call, and this can be very important. When you are worried about performance (execution time) you need to worry about more than just how long the program takes, you need to time every function.
  • The code does not use any standard C input or output so it does not need to include stdio.h.
  • The C++ programming language does not automatically initialize variabes to zero, compiling this code yields 3 warning messages about the public variables in the QuickProfiler not being initialized, there are 2 ways to initialize these variables, one is to provide a constructor, the other is to assign values in the declarations.
  • The program doesn't stop the timer if there is no path returned. It may be better to start the time after the return from the call to GetCurrentDirectory(MAX_PATH, currentDirectory); or to stop the time before the return.

Prefer C++ Casts Over Older C Style Casts

In the Stop() function the code has a C++ style cast and an older C style cast. Be consistent and use the proper forms in the language. Use a static_cast for the double.

        seconds = (double)millis / 1000.0;

versus

        seconds = static_cast<double>(millis / 1000.0);

DRY Code

There is a programming principle called the Don't Repeat Yourself Principle sometimes referred to as DRY code. If you find yourself repeating the same code multiple times it is better to encapsulate it in a function. If it is possible to loop through the code that can reduce repetition as well. This code in main repeats 3 times:

        sourcefile_extensions.insert(_T("cpp"));
        sourcefile_extensions.insert(_T("h"));
        sourcefile_extensions.insert(_T("hpp"));

There is actually no reason to repeat it, since that appears to be a default for all the options.

All the code really needs to check is if the program is recursive or non-recursive.

Standard Program Exit Values

In both C and C++ there are system defined program exit values, these are EXIT_SUCCESS and EXIT_FAILURE. In C++ these are always defined in standard C they can be accessed by including stdlib.h. It is better to use these symbolic constants that to use the raw numbers 0 and 1. They are more portable, while it is rare these days there may be operating systems that have different definitions for these constants.

Complexity

The functions int _tmain(), void CountLines(), and void ProcessFiles() are too complex (do too much). As programs grow in size the use of main()` should be limited to calling functions that parse the command line, calling functions that set up for processing, calling functions that execute the desired function of the program, and calling functions to clean up after the main portion of the program. All of these functions should be broken into smaller functions that perform just one action.

There is also a programming principle called the Single Responsibility Principle that applies here. The Single Responsibility Principle states:

that every module, class, or function should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by that module, class or function.

As promised, Alternate Code that Compiles

As I mentioned about part of the program did not compile for me in the newest version of VC++. Here is the update:

class QuickProfiler
{
public:
    std::chrono::high_resolution_clock::time_point t0;
    long long millis;
    double seconds;
    long long diff;

    inline void Start()
    {
        t0 = std::chrono::high_resolution_clock::now();
    }

    inline void Stop()
    {
        using namespace std::chrono;
        std::chrono::high_resolution_clock::time_point t1 = std::chrono::high_resolution_clock::now();
        millis = duration_cast<milliseconds>(t1 - t0).count();
        diff = (t1 - t0).count();
        seconds = (double)millis / 1000.0;

        seconds = static_cast<double>(millis / 1000.0);
    }
};
\$\endgroup\$
3

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.