HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Steam Account Switcher

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
switcheraccountsteam

Problem

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 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 = '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

Solution

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!

Code Snippets

wstring wParameters = L"-login " + wLogin + L" " + wPass, wSteam =  wSteamDir;
                                                          ^^^^^^
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);
}

Context

StackExchange Code Review Q#124764, answer score: 5

Revisions (0)

No revisions yet.