7
\$\begingroup\$

I've been to C++ lessons a long time ago and now I want to create a simple console program for switching Steam accounts. Please help me analyze my code and find my mistakes or better ways of doing something.

Main

#include "stdafx.h"
#include "functions.h"

using namespace std;

bool getNumberOrQuit(int &num, const int lcounter);

int main()
{
    //system("chcp 1251");
    //system("cls")
    SetConsoleTitle(L"Steam Account Switcher v0.02");

    bool repeatProgram = false, emergencyExit = 0;;
    string temp;
    map<int, string> login, pass;
    int lcounter, num;
    string::size_type pos = 0;
    wstring wSteamDir;

    do
    {
        screenClear();
        firstStart();   //First start with check
        lcounter = 0;   //Account counter

        if (repeatProgram == false)
        {
            temp = steamLocation();
            pos = temp.find("steam.exe");
            stringToWString(wSteamDir, temp.substr(0, pos));
        }
        repeatProgram = false;

        parseAccsFromDB(login, pass, lcounter); //Parsing accounts from accfile

        //Viewing accounts
        //----------------------------------------------------------------------------------------------
        screenClear();
        cout << "The list of accounts in datebase:\nType your account's number ('0' for more options, 'q' for exit)." << endl;
        showAccs(login, lcounter);      //Show accounts on the screen

        emergencyExit = getNumberOrQuit(num, lcounter); //Get user's choice of account or choice to quit the program
        if (emergencyExit) return 0;

        switch (num)
        {
            case 0: 
            {   //If user choose '0'
                showHelp(lcounter, login);
                repeatProgram = true;
                break;
            }
            default: 
            {
                launchSteam(login[num - 1], pass[num - 1], wSteamDir);
                break;
            }
        }
    } while (repeatProgram == true);
    return 0;
}

bool getNumberOrQuit(int &num, const int lcounter)
{
    char temp;
    do
    {
        temp = _getch();
        if (temp >= '0' && temp <= lcounter + '0')
        {
            num = temp - '0';
            return 0;
        }
        else if (temp == 'q')
        {
            return 1;
        }
    } while (temp < 0 || temp > lcounter);

    cout << "\nThe Application has encountered a critical error! (Error code: 11)\n";   //"ERRORCODE11"
    screenPause();
    return 1;
}

bool firstStart(bool override)
{
    CSimpleIniA ini;
    ini.SetUnicode();

    bool fs = true, iniExist;
    string fileEmpty;

    ifstream accFile("accounts");
    getline(accFile, fileEmpty);
    accFile.close();

    iniExist = iniExistCheck();
    ini.LoadFile("settings.ini");

    if (fileEmpty != "") fs = false;
    else if (!override)
    {
        fs = ini.GetBoolValue("General", "firststart");
    }
    //else if (fileEmpty != "" && override) fs = false;


    if (fs) //First time message and first ini setup
    {
        cout << "Welcome to Steam Account Switcher!\nI'm glad you're using my simple program.\nNow you have to add your steam account.\nEnter it below.\n";
        addAccount();
        screenClear();
    }

    ini.SetBoolValue("General", "firststart", false);
    ini.SaveFile("settings.ini");

    return fs;
}

Functions.h

#ifndef _ST_ACC_F_
#define _ST_ACC_F_
#pragma once

#include "SimpleIni.h"  //Ini File Read\Write Library

#include <fstream>
#include <iostream>
#include <string>
#include <conio.h>

using namespace std;

bool writeAccToDB(string login, string pass);   //Write in a file login and pass
bool addAccount();  //Message about addiding account and writing it
void runSteam(const string login, const string pass, const wstring wSteamDir);
void parseAccsFromDB(map<int, string> &login, map<int, string> &pass, int &lcounter);
void showAccs(map<int, string> &login, const int lcounter);
string getPassword();
bool deleteAccount(const int acc_num);
void showHelp(const int lcounter, map<int, string> &login);
bool firstStart(bool override = 0); //Firststart check and message
void stringToWString(wstring &ws, const string &s);
void launchSteam(const string login, const string pass, const wstring wSteamDir);
string steamLocation();
string getSteamLocation();
bool iniExistCheck();

void screenClear();
void screenPause(const string customMessage = "");
#endif // !_ST_ACC_F_

Steam_acc_funcs.cpp

#include "stdafx.h"
#include "functions.h"

using namespace std;

void runSteam(const string login, const string pass, const wstring wSteamDir)
{
    wstring wLogin, wPass;
    stringToWString(wLogin, login);
    stringToWString(wPass, pass);
    wstring wParameters = L"-login " + wLogin + L" " + wPass, wSteam =  wSteamDir;
    wSteam += L"steam.exe";

    const HRESULT hr = CoInitializeEx(NULL, COINIT_APARTMENTTHREADED | COINIT_DISABLE_OLE1DDE);
    ShellExecute(NULL, L"open", wSteam.c_str(), wParameters.c_str(), wSteamDir.c_str(), SW_SHOWMINIMIZED);
}

void stringToWString(wstring &ws, const string &s)
{
    wstring wsTmp(s.begin(), s.end());
    ws = wsTmp;
}

bool writeAccToDB(string login, string pass)
{
    fstream accfile("accounts", ios::out | ios::app);
    if (accfile << login << ";" << pass << endl)
    {
        accfile.close();
        return 1;
    }
    accfile.close();
    return 0;
}

bool addAccount()
{
    bool repeat = 1, add_result;
    string login, pass;
    char input;

    while (repeat == 1)
    {
        cout << "Login = ";
        cin >> login;

        cout << "Pass = ";
        pass = getPassword();

        add_result = writeAccToDB(login, pass);
        if (!add_result)
        {
            cout << "Application has encountered an error: Can not write the account's file! (Error Code: 1)" << endl;  //"ERRORCODE1"

            screenPause();
            return 0;
        }

        cout << "Account added succesfully!";
        cout << "\nAdd one more account?(1\\0 or y\\n)";
        do
        {
            input = _getch();
            switch (input)
            {
                case ('0'):
                {
                    repeat = 0;
                    break;
                }
                case ('n'):
                {
                    repeat = 0;
                    break;
                }
                case ('1'):
                {
                    repeat = 1;
                    break;
                }
                case ('y'):
                {
                    repeat = 1;
                    break;
                }
                default:
                    break;
            }
        } while (input != '0' && input != '1' && input != 'n' && input != 'y');
    }
    return 1;
}

string getPassword()
{
    string passString = ""; //holds the password string
    char ch = _getch();

    while (true)
    {
        if (ch == '\r' && passString != "") break;
        else if (ch == '\b' && passString != "")
        {
            //passString = passString.substr(0, passString.size() - 1);
            passString.pop_back();
            cout << "\b"
                << " "
                << "\b";
        }
        else if (ch != '\b')
        {
            cout << '*';
            passString += ch;
        }
        ch = _getch();
    }

    cout << endl;

    return passString;
}

void parseAccsFromDB(map<int, string> &login, map<int, string> &pass, int &lcounter)
{
    ifstream accfile("accounts");

    string::size_type pos = 0;
    string line;

    accfile.seekg(0);
    while (getline(accfile, line))   //Getting lines
    {
        pos = line.find(";");   //Locating sparator
        if (pos == string::npos) continue;   // For safety

        login[lcounter] = line.substr(0, pos);   //Setting login var    substr - to get line from specific pos
        pass[lcounter] = line.substr(pos + 1);   //Setting pass var

        lcounter++;
    }
    accfile.close();
}

void showAccs(map<int, string> &login, const int lcounter)
{
    for (int i = 0; i < lcounter;i++) 
        cout << i + 1 << "=" << login[i] << endl;
}

bool deleteAccount(const int acc_num)
{
    bool result = 0;
    ifstream accfile("accounts");
    ofstream new_accfile("newaccfile");
    string line;
    int lcounter = 0;

    if (!accfile.is_open()) return 0;
    else if (!new_accfile.is_open()) return 0;

    while (getline(accfile, line))
    {
        lcounter++;
        if (lcounter == acc_num) continue;
        new_accfile << line << endl;
    }
    accfile.close();
    new_accfile.close();
    remove("accounts");
    rename("newaccfile", "accounts");
    return 1;
}

void showHelp(const int lcounter, map<int, string> &login)
{
    screenClear();
    char input, input2;
    int input2_5;

    cout << "Type 1 to add a new account." << endl
        << "Type 2 to delete an account." << endl;

    do
    {
        input = _getch();
        switch (input)
        {
            case('1'):
            {
                cout << "Enter a new account below." << endl;
                addAccount();
                break;
            }
            case('2'):
            {
                cout << "Enter the number of the account you want to delete.\n";
                showAccs(login, lcounter);
                do
                {
                    input2 = _getch();
                    input2_5 = input2 - '0';
                    if (input2_5 >= 1 && input2_5 <= lcounter)
                    {
                        bool delResult = deleteAccount(input2_5);
                        if (delResult) cout << "Account \"" << login[input2_5 - 1] << "\" succesfully deleted." << endl;
                        else cout << "Error deleting account.";
                        screenPause("Press any key to return to main menu. . .");
                        break;
                    }
                    else cout << "Account with this number doesn't exist! Type a right one.\n";
                } while (input2_5 < 1 || input2_5 > lcounter);
                break;
            }
        //default:
        //{
        //  break;
        //}
        }
    } while (input != '1' && input != '2');
}

void launchSteam(const string login, const string pass, const wstring wSteamDir)
{
    system("taskkill /F /IM GameOverlayUI.exe > nul 2>&1");
    system("taskkill /F /IM Steam.exe > nul 2>&1");
    system("timeout /t 1 > nul 2>&1");
    runSteam(login, pass, wSteamDir);
}

string steamLocation()
{
    CSimpleIniA ini;
    ini.SetUnicode();
    ini.LoadFile("settings.ini");

    bool iniExist;
    iniExist = iniExistCheck();
    if (iniExist)
    {
        string locSet = ini.GetValue("Steam", "location");
        if (locSet != "") return locSet;
    }

    ifstream checkSteam("C:\\Program Files (x86)\\Steam\\steam.exe");
    if (checkSteam.good())
    {
        checkSteam.close();
        ini.SetValue("Steam", "location", "C:\\Program Files (x86)\\Steam\\steam.exe", NULL, true);
        ini.SaveFile("settings.ini");
        return "C:\\Program Files (x86)\\Steam\\steam.exe";
    }
    else
    {
        checkSteam.close();
        cout << "Steam isn't located in default location. Please, enter it manually.\n(Example: D:\\Programs\\Steam\\steam.exe)" << endl;
        return getSteamLocation();
    }
}

string getSteamLocation()
{
    ifstream checkSteam;
    CSimpleIniA ini;
    ini.SetUnicode();
    ini.LoadFile("settings.ini");

    string loc;
    bool customMessage = 0;

    do
    {

        if (customMessage) cout << "Steam isn't located in path you entered. Try again." << endl;
        //cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
        while (getline(cin, loc))
        {
            if (loc != "") break;
        }
        checkSteam.open(loc);
        if (checkSteam.good())
        {
            checkSteam.close();
            ini.SetValue("Steam", "location", loc.c_str(), NULL, true);
            ini.SaveFile("settings.ini");
            return loc;
        }
        customMessage = 1;
    } while (true);
}

bool iniExistCheck()
{
    CSimpleIniA ini;
    ini.SetUnicode();

    int iniExist = ini.LoadFile("settings.ini");

    if (iniExist < 0)
    {
        ini.SetBoolValue("General", "firststart", true);
        ini.SetValue("Steam", "location", "");
        ini.SaveFile("settings.ini");
        return 0;
    }
    return 1;
}

void screenClear()
{
    HANDLE hConsole;
    hConsole = GetStdHandle(STD_OUTPUT_HANDLE);
    COORD coordScreen = { 0, 0 };    // home for the cursor 
    DWORD cCharsWritten;
    CONSOLE_SCREEN_BUFFER_INFO csbi;
    DWORD dwConSize;

    // Get the number of character cells in the current buffer. 

    if (!GetConsoleScreenBufferInfo(hConsole, &csbi))
        return;
    dwConSize = csbi.dwSize.X * csbi.dwSize.Y;

    // Fill the entire screen with blanks.

    if (!FillConsoleOutputCharacter(hConsole, (TCHAR) ' ',
        dwConSize, coordScreen, &cCharsWritten))
        return;

    // Get the current text attribute.

    if (!GetConsoleScreenBufferInfo(hConsole, &csbi))
        return;

    // Set the buffer's attributes accordingly.

    if (!FillConsoleOutputAttribute(hConsole, csbi.wAttributes,
        dwConSize, coordScreen, &cCharsWritten))
        return;

    // Put the cursor at its home coordinates.

    SetConsoleCursorPosition(hConsole, coordScreen);
}

void screenPause(const string customMessage)
{
    if (customMessage == "") cout << "Press any key to continue . . .";
    else cout << customMessage;
    _getch();
}
\$\endgroup\$
0

2 Answers 2

4
\$\begingroup\$

You shouldn't ever do using namespace <blah> - it makes it easier to accidentally create name conflicts, and also harder for anyone reading the code to understand where something is coming from. You especially never want to do this in a header file - this will tend to cause a lot of problems, and would be very surprising to anyone using your code. If you don't want to prefix names with their namespace, a safer option is to use specific names, such as using std::vector;. This should still never happen in header files, however.

It is really weird to me that you initialize two bools, but one to false and the other to 0 - this doesn't really make sense (you also have an extra semicolon).

Instead of checking if a boolean is equal to false, just use !boolean.

I really don't like how you tend to create all of your variables at the top of your function - this is something that is done in C, but doesn't make sense in C++. Declare variables as close as possible to where they are used, and if you can use them in a scope (so they fall out of scope and can't be accidentally reused) then do so.

If you're going to mutate something in a function I strongly prefer passing by pointer than by reference, so you have the explicit & making it clear that something is going to be modified. This is stylistic preference, however, and not everyone feels the same way.

If you only have two branches, you don't need a switch statement.

Be consistent with when you use curly braces, and tend on the side of always using them even if syntactically they may be omitted.

Don't define macros with a leading _ - this is reserved for the compiler. It also doesn't make sense to use the #ifndef <blach> #define <blah> #endif pattern but also use #pragma once - pick one and stick with it, however I prefer the #ifndef approach (most compilers do support #pragma once, but why risk it).

In your error cases you print errors to cout - they should go to cerr. You should probably also return the error code you reference instead of 0. You may also want to exit(ERR_CODE) if it isn't recoverable. Typically 0 is reserved for the success case.

Overall I think your main issues are stylistic - the code could be cleaner and easier to read.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you very much for the answer, it helped me a lot. And sorry for not answering in time, I just checked the answers, resolved all the mistakes and completely forgot to reply. But better later than never, isn't it? :) \$\endgroup\$
    – Gray K
    Dec 2, 2016 at 19:57
5
\$\begingroup\$

In addition to what was already said, other cleanups that you could apply:

  • Consider consolidating some of your hardcoded constants into consts. In special, the names of files that you reference through the code. For instance, "settings.ini" is referenced in more than one place. It should be a static const std::string at the global level, so if you change the name of that file, you don't have to search and replace in a bunch of different places. This path: "C:\\Program Files (x86)\\Steam\\steam.exe" is also references three or four times. Make it a named constant string instead.

  • Don't mix bool with integers. Take a look at deleteAccount for example. The function returns bool, but all return statements are returning ints. This happens in pretty much all functions returning bool. You want to return true or false instead.

  • The HRESULT of CoInitializeEx is set but never used, which is odd. Either ignore the return completely if you don't care or check it and take an appropriate action (probably logging the error and quitting). Same goes for ShellExecute. I can think of many cases in which it can fail (think about user permissions). You should check its result for failures.

  • Please avoid declaring multiple variables in one line. If you get into the habit of declaring them at the first usage, rather then at the top of a function, most of this problem goes away naturally. For instance, very hard to notice there's another string being declared here at the very end:

    wstring wParameters = L"-login " + wLogin + L" " + wPass, wSteam =  wSteamDir;
                                                              ^^^^^^
    
  • You'll generally want to pass your strings by const reference to avoid unnecessary copies. Remember that in C++ the default for function parameters is by-value, creating a local copy, even for class types. There are a few cases where you can replace with const &.

  • In parseAccsFromDB you didn't check if the file was opened successfully. Likely an oversight.

  • You can rely on the stream destructor to close the files for you. No need to sprinkle file.close() calls all over the place.

  • This seems like a major security hole on your application. The system function is very unsafe, because it just attempts to execute whatever command you pass to it. I would consider a replacement probaly using ShellExecute or a variation thereof.

    void launchSteam(const string login, const string pass, const wstring wSteamDir)
    {
        system("taskkill /F /IM GameOverlayUI.exe > nul 2>&1");
        system("taskkill /F /IM Steam.exe > nul 2>&1");
        system("timeout /t 1 > nul 2>&1");
        runSteam(login, pass, wSteamDir);
    }
    

All in all, the issues are mostly aesthetic, which hurt readability. I would also like to commend you for not making use of global variables and using function parameters instead. Good job on that!

\$\endgroup\$
1
  • \$\begingroup\$ Thank you, and sorry for not answering in time, I completely forgot about it. The reason I choose to use system("...") is because I couldn't find a good way to kill steam without all the hassle with windows api and handles. The same reason goes to timeout. \$\endgroup\$
    – Gray K
    Dec 2, 2016 at 20:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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