2
\$\begingroup\$

Is there a way I could improve my code's readability, or have I finally won the war against sloppy coding?

Here's a game launcher I made for The Sims 3 to fix the awful performance caused by big cache files.

using System.Collections.Generic;
using System.Diagnostics;
using System.Windows.Forms;

namespace Sims_3___Anti_Cache
{
    public partial class Form1 : Form
    {
        Game Sims3;

        public Form1()
        {
            InitializeComponent();

            Sims3 = new Game();
            Sims3.cache = new List<string>() { "CASPartCache.package", "compositorCache.package", "scriptCache.package",
                                               "simCompositorCache.package", "socialCache.package" };

            Sims3.cache_path = "C:/Users/DarkShadow/Documents/Electronic Arts/The Sims 3/";
            Sims3.clear_cache();
            Sims3.path = "C:/Program Files (x86)/Electronic Arts/The Sims 3/Game/Bin/TS3.exe";

            isCleared.Enabled = true;
        }

        private void isCleared_Tick(object sender, System.EventArgs e)
        {
            if (Sims3.cache_cleared)
            {
                Process.Start(Sims3.path);
                Application.Exit();
            }
        }

    }
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ This is quite simple code, there isn't much to review here. Why didn't you include the Game class too? \$\endgroup\$
    – svick
    Commented Jan 1, 2014 at 16:29
  • \$\begingroup\$ Because there's nothing special about it. All it does is store values, check if the file in cache exist and delete it. \$\endgroup\$
    – user34261
    Commented Jan 1, 2014 at 16:48
  • 1
    \$\begingroup\$ Was the title misleading? We generally like to have titles that reflect the code's purpose. This helps increase visibility on the site and on search engines. \$\endgroup\$
    – Jamal
    Commented Jan 1, 2014 at 17:03

2 Answers 2

4
\$\begingroup\$

You would benefit from removing the hard-coded strings out of the code. Best to put them in an app.config file or a resource file. The config file has the additional advantage that the strings could be changed by the user without recompiling.

The naming of the methods and properties is not standard for C#. Use Pascal case for public methods and properties with no underbar, e.g.

Sims3.CachePath = configCachePath;
Sims3.ClearCache();

The naming of local variables and private methods uses camel case, e.g.

isClearedTick();
\$\endgroup\$
1
\$\begingroup\$

What is the Game class? What does it do?

Some issues though:

  • hard-coding the locations and names of the files is a problem. At a minimum you should make the code more portable by making those values configurable (command-line, using variables like PROGRAMFILES(X86) and CSIDL_MYDOCUMENTS).
  • you do not do any validation to ensure that the data actually exists.

From the code you show, this launcher does not do very much, it clears a cache (In the background?) and (on a call-back) starts a child process.

Is there any reason you need to create a full program for this? A Batch file seems like the right tool (maybe you need a program to clear the actual cache files?):

@echo off
rem clear cache files
ClearCaches "%CSIDL_MYDOCUMENTS%\Electronic Arts\The Sims 3\*Cache.package"
rem start game
"%PROGRAMFILES(X86)%\Electronic Arts\The Sims 3\Game\Bin\TS3.exe"

P.S. After Googling I found this advice:

When you click the batch file it will run and delete the four cache files then run the Sims.

@echo off
del "C:\Documents and Settings\My Computer\My Documents\Electronic Arts\The Sims 3\CASPartCache.package"
del "C:\Documents and Settings\My Computer\My Documents\Electronic Arts\The Sims 3\scriptCache.package"
del "C:\Documents and Settings\My Computer\My Documents\Electronic Arts\The Sims 3\compositorCache.package"
del "C:\Documents and Settings\My Computer\My Documents\Electronic Arts\The Sims 3\simCompositorCache.package"
start /d "C:\Program Files\Electronic Arts\The Sims 3 Ambitions\Game\Bin" Sims3Launcher.exe
\$\endgroup\$
1
  • \$\begingroup\$ While I agree with the intent of %CSIDL_MYDOCUMENTS%, it doesn't appear to be defined for me. Instead the closest I can suggest for a batch file appears to be %USERPROFILE%\Documents\... or via registry queries \$\endgroup\$ Commented Jan 1, 2014 at 15:47

Your Answer

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

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