1
\$\begingroup\$

I'm learning the file system library and thought nothing could be better than a project, so I thought of recreating the basic Linux commands such as ls, cd etc using C++ and <filesystem>.

As I said, I'm learning C++ and exploring libraries, so there could be weird code and all those things; could you review my code and say where I can improve the code?

My Code:

main.cpp

#include <iostream>
#include <filesystem>
#include "command.h"
namespace fs = std::filesystem;
int main()
{
    //Basic Linux Feel ;)
    std::string CurPath = fs::current_path();
    std::cout << "~";
    color::color_blue(CurPath);
    std::cout << "$ ";
    //Main Loop
    while (true)
    {
        std::string cmnd;
        getline(std::cin, cmnd);
        #if DEBUG
                if (cmnd == "exit")
                    return 0;
        #endif
        DetermineCommand(cmnd);
    }
}

command.h

#include <iostream>
#include <fstream>
#include <sstream>
#include <vector>
#include <chrono>
#include <experimental/filesystem>
namespace fs = std::experimental::filesystem;
using namespace std::chrono_literals;
//DEBUG Mode :) I Used This For Exitting The Infinite Loop
#define DEBUG 1

//Used For Setting Color To Console According To OS
class color
{
  public:
    static void color_green(std::string path)
    {
#ifndef __LINUX
        //color code for green
        std::cout << "\033[32m" << path << "\033[0m";
#else
        //Green
        system("Color 02");
        std::cout << path;
        //White
        system("Color 07");
#endif
    }
    static void color_blue(std::string path)
    {
//color code for blue
#ifndef __LINUX
        std::cout << "\033[34m" << path << "\033[0m";
#else
        //Blue
        system("Color 01");
        std::cout << path << " ";
        //White
        system("Color 07");
#endif
    }
};
//Says The Current Path Color Encoded
void SayPath()
{
    std::cout << std::endl;
    std::string CurPath = fs::current_path();
    std::cout << "~";
    color::color_blue(CurPath);
    std::cout << "$ ";
}
//cp Used For Copying Files
void cp(std::string p1, std::string p2)
{
    fs::path ToCopy = p1, Target = p2;
    if (!fs::exists(p2)|| !fs::exists(p1))
    {
        throw std::string("cp: cannot stat " + p1 + ": No such file or directory");
    }
    //Trying To Warn
    fs::path CheckExist = p2 + "/" + p1;
    if (exists(CheckExist))
    {
        std::cout << "The File Already Exist On The Given Path Replace Them [Y/N] ";
        char ans;
        std::cin >> ans;
        if (ans != 'Y' || ans != 'y')
        {
            std::cout << "Aborted!";
            return;
        }
    }
    fs::copy(p1, p2, fs::copy_options::recursive);
}
//mkdir Used For Making Directories
void mkdir(std::string &s)
{
    fs::create_directories(s);
}

//rm Used For Deleteing Files
void rm(std::string s)
{
    std::error_code err;
    //Handling The Case If Directory Doesn't Exsist
    if (!fs::exists(s))
    {
        throw std::string("rm: cannot remove '" + s + "': No such file or directory");
    }
    if (fs::is_directory(s, err))
    {
        if (fs::exists(s))
        {
            throw std::string("rm: cannot remove '" + s + "': Is a directory");
        }
    }
    fs::remove(s);
    return;
}

//The “cd” Command Used For Navigating Over Directories
void cd(std::string &path)
{
    //cd .. implementation - going to parent directory
    if (path == "..")
    {
        fs::path Parent = fs::current_path().parent_path();
        fs::current_path(Parent);
        return;
    }
    //If The User Entered A Blank Path Or Just The "cd" command
    else if (path == "")
    {
        fs::current_path(fs::current_path().root_directory());
        return;
    }
    //If Path Doesn't Exist
    else if (!fs::exists(path))
    {
        throw std::string("cd: " + path + " No such file or directory");
    }
    else
    {
        fs::current_path(path);
    }
}

//The “ls” Command Is Used For Printing All The Files And Directories In The Cirrent Folder
void ls()
{
    fs::path CurPath = fs::current_path();
    //f_name will be used for storing paths and names
    std::string f_name;
    for (auto &Name : fs::directory_iterator(CurPath))
    {
        //If It's A File
        std::error_code err;
        if (fs::is_regular_file((Name).path(), err))
        {
            f_name = Name.path().filename();
            color::color_green(f_name);
            std::cout << " ";
        }
        //If It's A Folder
        else
        {
            f_name = Name.path().filename();
            color::color_blue(f_name);
            std::cout << " ";
        }
    }
}
//mv move command
void mv(std::string From, std::string To, bool IsRename)
{
    fs::path p1 = From, p2 = To;
    //Checking If It's For Renaming
    if (!IsRename)
    {
        fs::path CheckExist = To + "/" + From;
        if (exists(CheckExist))
        {
            std::cout << "The File Already Exist On The Given Path Replace Them [Y/N] ";
            char ans;
            std::cin >> ans;
            if (ans != 'Y' || ans != 'y')
            {
                std::cout << "Aborted!";
                return;
            }
        }
    }
    fs::copy(From, To);
    fs::remove(From);
}
void rmdir(std::string Path){
    if(!fs::exists(Path)){
        throw std::string("rmdir: failed to remove "+Path+": No such file or directory");
    }
    if(!fs::is_directory(Path)){
        throw std::string("rmdir: failed to remove "+Path+": Not a directory");
    }
    fs::remove(Path);
}
//Related To File Information
std::streampos GetSize(std::string PathName){
    std::streampos FullSize=0;
    fs::path Path=PathName;
    if(fs::is_directory(Path)){
        for (auto& Name:fs::recursive_directory_iterator(Path)){
            if(fs::is_regular_file(Name.path())){
                std::ifstream Open(Name.path(),std::ios_base::binary);
                std::streampos ThisSize=Open.tellg();
                Open.seekg(0,std::ios::end);
                FullSize+=ThisSize-Open.tellg();
                Open.close();
            }
        }
        return FullSize;
    }
    else{
        std::ifstream Open(Path);
        std::streampos ThisSize=Open.tellg();
        Open.seekg(0,std::ios::end);
        ThisSize=ThisSize-Open.tellg();
        Open.close();
        return ThisSize;
    }
}
void PrintTime(std::string Path){
    //Copied From Runebook.dev ;)
    auto ftime=fs::last_write_time(Path);
    std::time_t cftime = decltype(ftime)::clock::to_time_t(ftime);
    std::cout <<std::asctime(std::localtime(&cftime));
}
void GetInfo(std::string Path){
    if(!fs::exists(Path)){
        throw std::string("--info: "+Path+" No such file or directory");
    }
    std::streampos Size=GetSize(Path);
    std::cout<<"SIZE : "<<Size<<std::endl;
    std::cout<<"PATH : "<<Path<<std::endl;
    std::cout<<"LAST MODIFIED : "; 
    PrintTime(Path);
    if(fs::is_directory(Path)){
        std::cout<<"TYPE : Directory";
    }
    else{
        std::cout<<"TYPE : File";
    }
}
void cat(std::string Path){
    if(!fs::exists(Path)){
        throw std::string("cat: "+Path+": No such file or directory");
    }
    else if(fs::is_directory(Path)){
        throw std::string("cat: "+Path+": Is a directory");
    }
    std::string Temp;
    std::ifstream Reader(Path);
    while(getline(Reader,Temp)){
        std::cout<<Temp<<std::endl;
    }
}
void help(){
    std::cout<<"•cd\n•ls\n•mkdir\n•rmdir\n•rm\n•pwd\n•echo\n•clear\n•cp\n•mv\n•rename\n•--info\n•cat";
    SayPath();
}
//This Function Is Used For Determining The Type Of Command
void DetermineCommand(std::string &s)
{
    std::stringstream Extractor(s);
    std::string Type;
    Extractor >> Type;
    if (Type == "cd")
    {
        if (s.length() == 2 || s[3] == ' ')
        {
            std::string Empty = "";
            cd(Empty);
            SayPath();
            return;
        }
        //I'm Reusing The Type Variable To Store The Path As The Next Would Be The Path
        Extractor >> Type;
        try
        {
            cd(Type);
            SayPath();
        }
        catch (std::string Error)
        {
            std::cout << Error;
            SayPath();
            return;
        }
    }
    else if (Type == "ls")
    {
        ls();
        SayPath();
    }
    else if (Type == "mkdir")
    {
        std::string Path;
        Extractor >> Path;
        mkdir(Path);
        SayPath();
    }
    else if (Type == "rm")
    {
        std::string Parms;
        Extractor >> Parms;
        try
        {
            rm(Parms);
            std::cout << "\b";
        }
        catch (std::string Error)
        {
            std::cout << Error;
        }
        SayPath();
    }
    else if (Type == "pwd")
    {
        std::cout << fs::current_path();
        SayPath();
    }
    else if (Type == "echo")
    {
        std::string s;
        std::string a;
        Extractor>>s;
        while(Extractor>>a){
            s+=a+' ';
        }
        if(s[0]=='"' && s[s.length()-1]=='"'){
            s.erase(s.begin());
            s.erase(s.end());
        }
        std::cout << s;
        SayPath();
    }
    else if (Type == "clear")
    {
        system("clear");
        SayPath();
    }
    else if (Type == "cp")
    {
        std::string p1, p2;
        Extractor >> p1 >> p2;
        try
        {
            cp(p1, p2);
        }
        catch (std::string Err)
        {
            std::cout << Err;
        }
        SayPath();
    }
    else if (Type == "mv")
    {
        std::string p1, p2;
        Extractor >> p1 >> p2;
        try{
            mv(p1, p2, false);
        }
        catch(...){
            std::cout<<"Some Error Occured\n";
        }
        std::cout<<'\b';
        SayPath();
    }
    else if (Type == "rename")
    {
        std::string p1, p2;
        Extractor >> p1 >> p2;
        mv(p1, p2, true);
        std::cout<<'\b';
        SayPath();
    }
    else if(Type=="--info"){
        try{
            std::string Path;
            Extractor>>Path;
            GetInfo(Path);
            //Flushing \n
            std::cout<<'\b';
            SayPath();
        }
        catch(std::string err){
            std::cout<<err;
            SayPath();
        }
    }
    else if(Type == "cat"){
        std::string FName;
        Extractor>>FName;
        try{
            cat(FName);
        }
        catch(std::string err){
            std::cout<<err<<std::endl;
        }
        std::cout<<'\b';
        SayPath();
    }
    else if(Type=="--help"){
        help();
    }
    else if(Type == "rmdir"){
        std::string Path;
        Extractor>>Path;
        rmdir(Path);
        SayPath();
    }
    else{
        std::cout<<"No such command found use --help to get a list of all command";
        SayPath();
    }
}

I have uploaded it on GitHub Too - Repo.

Note - I didn't implement all options of every command; for example, I just implemented ls but not ls -a etc.

Thanks for checking this out!!

\$\endgroup\$
6
\$\begingroup\$

Command Loop

    std::cout << "$ ";
    //Main Loop
    while (true)
    {
        std::string cmnd;
        getline(std::cin, cmnd);
        #if DEBUG
                if (cmnd == "exit")
                    return 0;
        #endif

A few comments here:

  • You output a prompt, "$ ", but you only do that once. The other times, that is done in SayPath(). This isn't very transparent.
  • If you pipe input from a file into this program, getline() will fail after reading the last line. You never check for success. Use while (getline(cin, cmnd)) instead.
  • Why does this behave differently when DEBUG is defined?
  • Consider not using a command loop, because it doesn't lend itself to reuse in scripts, only for interactive use. Instead, take the (single!) command via commandline. You would then call your program like ./youprog ls /etc to list the contents of the /etc dir.

Colours

You have code that creates coloured output, which you run conditionally using #ifdef __LINUX. However, that's the wrong approach! For one, you assume that when it runs on Linux, you want coloured output. However, what if you pipe the output of this program into another program. In that case, you'd have the escape sequences in your output which the other program receives as input.

Another thing is that this is not a Linux specialty! Instead, it depends on the shell and the terminal. Since you can't know either when compiling the program, you need a solution that makes the decision at runtime instead.

Class vs. Namespace

You have a class color, but you never create an instance of that class. Instead, you use the static functions that this class provides. This should be a simple namespace instead.

Header vs. Implementation

Your approach to move some code to a separate file is not uncommon. However, you typically have a header file (typically *.h or *.hpp) and an implementation file (typically *.cpp). The header file also has so-called include guards. This is a wide topic but keep your eyes open, you will find lots of examples for it.

Else After Return

You have lots of code like

if (...)
{
    ...
    return;
}
else
{
    ...
}

You can save yourself the else completely. As a rule of thumb, use if-else only when the paths converge after the else branch. If you return, the path ends and it will never converge with the other path. Another example is your big command dispatch function, where you have this structure:

if (Type == "cd")
{
    ...
}
else if (Type == "ls")
{
    ...
}
else if (Type == "rm")
{
    ...
}
else if (Type == "cd")
{
    ...
}
else
{
    throw ...
}

Here, the paths actually converge, but the converging part is empty. So, if you want to understand what the "cd" does, you will have to scroll down past all other handlers and you will find that nothing else happens! This would be immediately obvious if you returned instead.

Note that not just return ends the path you can take through a function. Other such flow control statements are

  • throw
  • break
  • continue
  • goto
  • exit()
  • abort()
  • longjmp()

These all make any else branch redundant.

Throwing Strings

Firstly, apart from one or two places, you approach of using exceptions to signal errors looks good. Don't waste your time with returncodes unless you have a good reason. However, don't throw std::string instances. Instead, use std::runtime_error, which takes a string in its constructor. Of course, you have to adjust any catch clauses as well accordingly:

    try
    {
        ...
    }
    catch (std::exception const& e)
    {
        std::cout << e.what() << std::endl;
        ...
    }

Note that you catch the exception using a const reference (not by value) to the baseclass. If you have specific exceptions for specific errors that need different processing, you can use those specific types instead. For output of the error message, you retrieve the error string using e.what().

Linebreaks

The default output stream std::cout is buffered. If you write to it using cout << "foo", nothing is actually displayed! Only when the output is flushed, it is actually printed (e.g. on screen). It is implicitly flushed when you retrieve input from std::cin, so this might not be immediately obvious. Another way to flush the stream is to write std::endl to it, which also finishes the line. I'd suggest you use that by default.

Parameter Passing

You have a bunch of handler functions. Some of them take a string as parameter, but you're not consistent there. In some cases, you pass the string by value (std::string p), in other cases, you pass a reference (std::string& p). The third variant of passing a reference to a constant (std::string const& p) is unused. The differences should be explained in your C++ tutorial.

One thing which might not be explained in every tutorial: When you pass a reference to a non-constant, the implicitation is that the function is supposed to modify the value. Don't use this if the function doesn't modify the value. Also, if you use that, document in what way the value is modified. This is important for others to use the function correctly, but it also helps you structure your thoughts when writing it, like every documentation.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Thanks man for taking your time I'll keep those things in mind :) \$\endgroup\$
    – Agent_A
    May 26 '21 at 14:29
  • 1
    \$\begingroup\$ If you refactor to perform a single operation per process, then cd won't be any use (because the process with the changed working directory exits before you can do anything with it). cd really is a shell operation. \$\endgroup\$ May 26 '21 at 14:34
  • \$\begingroup\$ cd is one such thing. Also, shell variables and environment variables. In e.g. Bash, call command -V cd to find out where something comes from. Another thing done by the shell instead of the called program is globbing. If you type ls * in the shell, the shell substitutes * with your directory content and then invokes ls with these arguments. Also handled by the shell are quotes (" and '). \$\endgroup\$
    – uli
    May 26 '21 at 15:10
2
\$\begingroup\$
    std::cout << "\033[32m" << path << "\033[0m";

Please don't do that - not all the world is an ANSI terminal.

\$\endgroup\$
2
  • \$\begingroup\$ Actually i wrote more than half of the code and this came in my made so in order to use curses I'll have to rewrite it completely that's the reason i did that. \$\endgroup\$
    – Agent_A
    May 26 '21 at 8:12
  • \$\begingroup\$ Eh? Just produce ordinary text output and it will work everywhere. If you don't implement -a, there's really no reason to force --color... \$\endgroup\$ May 26 '21 at 8:28
1
\$\begingroup\$

When you use conditional code make it functional so the code you read looks normal. i.e. don't mess up the code by putting conditional compilation in. Use conditional compilation to generate alternative versions of appropriate functions.

    static void color_green(std::string path)
    {
#ifndef __LINUX
        //color code for green
        std::cout << "\033[32m" << path << "\033[0m";
#else
        //Green
        system("Color 02");
        std::cout << path;
        //White
        system("Color 07");
#endif
    }
    static void color_blue(std::string path)
    {
//color code for blue
#ifndef __LINUX
        std::cout << "\033[34m" << path << "\033[0m";
#else
        //Blue
        system("Color 01");
        std::cout << path << " ";
        //White
        system("Color 07");
#endif
    }

This is hard to read because each block contains a lot of conditional code.

#ifndef __LINUX
    static const_expr char COLOR_GREEN[] = "32m";
    static const_expr char COLOR_WHITE[] = "0";
    // Now when you get comments from "Toby" you only have
    // fix the code in a single place and not modify the code
    // in multiple locations through the code base.
    void set_ink(char const[] color)  {std::cout << "\033[" << color << "m";}
#else
    static const_expr char COLOR_GREEN   = "color 3";
    static const_expr char COLOR_GREEN   = "color 7";
    void set_ink(char const[] color)  {system(color);}
#endif

static void color_green(std::string path)
{
    set_ink(COLOR_GREEN);
    std::cout << path;
    set_ink(COLOR_WHITE);
}
static void color_blue(std::string path)
{
    set_ink(COLOR_BLUE);
    std::cout << path;
    set_ink(COLOR_WHITE);
}

Now do it properly to make sure the color is reset even with exceptions:

class Ink
{
    Ink(char const[] color) {
        set_ink(color);
    }
    ~Ink() {
        set_ink(COLOR_WHITE);
    }
};
static void color_green(std::string path)
{
    Ink  ink(COLOR_GREEN);
    std::cout << path;
}
static void color_blue(std::string path)
{
    Ink  ink(COLOR_BLUE);
    std::cout << path;
}
\$\endgroup\$
1
  • \$\begingroup\$ Thanks I'll try writing neat and readable code \$\endgroup\$
    – Agent_A
    May 27 '21 at 23:03

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.