Reading code off the internet sometimes triggers me. I immediately start thinking: "hmm, why didn't you that, or that?". (I'm not trying to be that smartass or anything, and I'm by any means not the best, just wanted to make things a little clearer, okay, good).
Before we dive in, when writing something, always ask yourself these questions:
- What do I want to know?
- What do I already know?
Trust me, it's extremely helpful.
Here're a couple of dilemmas with examples, and more compact solutions:
1. A guy named Jerry wants a specific command to output the client's team. This is how he did it:
Spoiler
PHP Code:
#include <sourcemod>
#pragma semicolon 1
public void OnPluginStart() { RegConsoleCmd("sm_test", Cmd_Test); }
public Action Cmd_Test(int iClient, int iArgs) { // Validate the client. if (0 < iClient <= MaxClients && IsClientInGame(iClient)) { // If the clients team index is greater than two, i.e, if the client isn't a spectator. if (GetClientTeam(iClient) > 2) { // If the client is in the Counter-Terrorist team. if (GetClientTeam(iClient) == 3) PrintToChat(iClient, "Yay, you're in the Counter-Terrorist team!");
// If the client is in the Terrorist team. else if (GetClientTeam(iClient) == 2) PrintToChat(iClient, "Yay, you're in the Terrorist team!"); } }
return Plugin_Handled; }
Now, this code is perfectly fine, but. We can shorten it... a lot:
Spoiler
PHP Code:
#include <sourcemod>
#pragma semicolon 1
public void OnPluginStart() { RegConsoleCmd("sm_test", Cmd_Test); }
public Action Cmd_Test(int iClient, int iArgs) { // Validate the client. if (0 < iClient <= MaxClients && IsClientInGame(iClient)) { // We assign the team index to a variable, to save some space. int iTeam = GetClientTeam(iClient);
// If the clients team index is greater than two, i.e, if the client isn't a spectator. // We can then just use the ternary operator and we save lots of space! if (iTeam > 2) PrintToChat(iClient, "Yay, you're in the %sTerrorist team!", iTeam == 3 ? "Counter-" : ""); }
return Plugin_Handled; }
2. This one day I was writing a stats plugin and there was this one command that was supposed to create a menu, showing you the top players on the server, based different categories. Here's how I first did it:
Spoiler
PHP Code:
public void Menu_Top_Menu(int iClient) { Menu hMenu = new Menu(Menu_Top_Menu_Handler);
public int Menu_Top_Menu_Handler(Menu hMenu, MenuAction hAction, int iClient, int iMenu_Param) { if (IsValidClient(iClient)) { switch (hAction) { case MenuAction_Select: { char sQuery[256];
switch (iMenu_Param) { case 0: Format(sQuery, sizeof(sQuery), "SELECT * FROM user_stats ORDER BY exp DESC");
case 1: Format(sQuery, sizeof(sQuery), "SELECT * FROM user_stats ORDER BY kills DESC");
case 2: Format(sQuery, sizeof(sQuery), "SELECT * FROM user_stats ORDER BY headshots DESC");
case 3: Format(sQuery, sizeof(sQuery), "SELECT * FROM user_stats ORDER BY noscopes DESC"); }
public int Menu_Top_Menu_Handler(Menu hMenu, MenuAction hAction, int iClient, int iMenu_Param) { if (IsValidClient(iClient)) { switch (hAction) { case MenuAction_Select: { char sColumn[12];
Now, this code is perfectly fine, but. We can shorten it... a lot:
It would have been even 'shorter', if you didn't create a new "iTeam" variable, and simply used:
like in the first example.
The ternary operator:
PHP Code:
iTeam == 3 ? "Counter-" : ""
isn't always as useful, such as for example if the code you want to act based on the result of the condition is spanning more than one line, in these situations the classic if-then-else conditions may be more appropriate (and make the code more readable in the end).
__________________
Mostly known as "DarkDeviL".
Dropbox FastDL: Public folder will no longer work after March 15, 2017!
For more info, see the [SRCDS Thread], or the [HLDS Thread].
PrintToChat(iClient, "Yay, you're in the %sTerrorist team!", iTeam == 3 ? "Counter-" : "");
That is perfectly fine up until you want localizations (check out the Russian localization of team names on CS:GO), or different phrases depending on the team. At the very least I'd rather keep the entire team name in the third parameter to avoid these sorts of phrase concatenations.
Additionally, Menu_Top_Menu_Handler is in poor form -- the third parameter is not always going to be a client index, per the MenuAction documentation. You've gated off MenuAction_End, where the parameter in question is the MenuEnd reason, and so you're now always leaking handles (because the values that you think are clients are either 0 or negative). That said, I'd also take that approach to simplifying the query setup.
As far as clear code goes, I'm a fan of "Avoid Else, Return Early" to keep indentation to a minimum, but that's mostly a matter of opinion and not a strict style to adhere to.