Raised This Month: $ Target: $400
 0% 

How to improve this code?


Post New Thread Reply   
 
Thread Tools Display Modes
Author Message
pob18
Member
Join Date: Jun 2014
Old 07-02-2014 , 07:35   How to improve this code?
Reply With Quote #1

I got this code that reads steam ids from a config file.
After that when you plant/defuse the bomb, the plugin will check the steamids stored in the cfg and print in chat a message.

Like:
your steamidxxx -> [tag/etc] Player planted the bomb
non steamidxxx from -cfg -> Player planted the bomb...

But what if I want to use this for other functions? I always need to copy/paste the same function?
Isn't this a mistake?

How to improve this?

PHP Code:
#include <amxmodx>
#include <amxmisc>
#include <cstrike>
#include <csx>

#include <colorchat>

new Trie:g_tAuthIdOfPeople
#define FILE    "/Steams.cfg"
#define cm(%0)  ( sizeof(%0) - 1 )  

public plugin_init()
{
    
g_tAuthIdOfPeople TrieCreate()
    
ReadFile()
}

public 
ReadFile()
{
    new 
szFilePath128 ]
    
get_configsdirszFilePathcmszFilePath ) )
    
addszFilePathcmszFilePath ), FILE )

    new 
fopenszFilePath"rt" )
    
    if( !
)
    {
        new 
szMessage256 ]
        
formatexszMessagecmszMessage ), "Unable to open %s"szFilePath )
        
set_fail_stateszMessage )
    }
    
    new 
szData128 ]
    new 
szAuthID35 ]
    new 
szDummy]
    
    while( !
feof) )
    {
    
fgetsfszDatacmszData ) )
        
    if( !
szData] || szData] == ';' || szData] == '/' && szData] == '/' )
    {
        continue
    }

    
trimszData )
    
parseszDataszAuthIDcmszAuthID ), szDummycmszDummy ) )
        
    
TrieSetCellg_tAuthIdOfPeopleszAuthID)
        
    }
    
    
fclose)
}

public 
bomb_planted(index)
{
    new 
szAuthID35 ]; get_user_authidindexszAuthIDcmszAuthID ) )
    new 
iDummy;
    
    new 
iPlayers32 ], iNumiPlayers;
    
get_playersiPlayersiNum"ceh""TERRORIST" )
    
    for( 
0iNumi++ )
    {
    
Players iPlayers];
        if( 
TrieGetCellg_tAuthIdOfPeopleszAuthIDiDummy ) )
        {
        
ColorChat(PlayersGREEN"[tag/etc] %s ^1planted the bomb"get_nick(index) )
        }   
        
        else
        
ColorChat(PlayersGREEN"%s ^1planted the bomb"get_nick(index) )
    }
}

public 
bomd_defused(index)
{
                    
// Paste the code below again? This is not the best way right?
                    // How to improve?
}

get_nick( const index )
{
    new 
szName32 ]; get_user_nameindexszNamecmszName ) );
    return 
szName;

pob18 is offline
Arkshine
AMX Mod X Plugin Approver
Join Date: Oct 2005
Old 07-02-2014 , 08:59   Re: How to improve this code?
Reply With Quote #2

If you want to call the same code again, you just need to create a private function with the shared code, and calling it in both forward.

And by the way:

Trim() should be called before the check.
TrieGetCell() -> TrieKeyExists ; since you don't need associated value.
Call TrieDestroy() on plugin_end() forward.
set_fail_state() can format string, but not sure in what AMXX version it's available.
__________________
Arkshine is offline
HamletEagle
AMX Mod X Plugin Approver
Join Date: Sep 2013
Location: Romania
Old 07-02-2014 , 10:03   Re: How to improve this code?
Reply With Quote #3

Quote:
Originally Posted by Arkshine View Post
If you want to call the same code again, you just need to create a private function with the shared code, and calling it in both forward.

And by the way:

Trim() should be called before the check.
TrieGetCell() -> TrieKeyExists ; since you don't need associated value.
Call TrieDestroy() on plugin_end() forward.
set_fail_state() can format string, but not sure in what AMXX version it's available.
Quote:
Originally Posted by ConnorMcLeod View Post
Tries don't need to be destroyed at plugin_end, it is automatic.
So,who is wrong ? I just want to know the correct way.
HamletEagle is offline
Arkshine
AMX Mod X Plugin Approver
Join Date: Oct 2005
Old 07-02-2014 , 11:43   Re: How to improve this code?
Reply With Quote #4

With 1.8.2, It's designed like: at map change the handle system is well freed, but not the actual allocated memory from plugin datas, which will be reused (or extented) if needed. In others words, if you allocate x bytes from plugin A, and this plugin is disabled on next map, you will have x bytes allocated for nothing.

With 1.8.3, the trie natives are no more based on KTrie library but on Hashmap instead, the whole system is different and this time, memory should be freed properly at each map change.

Still, it's a good practice to free always when you can something you have previously allocated.
I would recommend to use always in this context TrieDestroy in plugin_end().
__________________
Arkshine is offline
Backstabnoob
BANNED
Join Date: Feb 2009
Location: Iwotadai Dorm
Old 07-02-2014 , 12:10   Re: How to improve this code?
Reply With Quote #5

I don't know what you're doing within the loop in bomb_planted, but it's wrong.
Backstabnoob is offline
Reply



Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump


All times are GMT -4. The time now is 21:17.


Powered by vBulletin®
Copyright ©2000 - 2024, vBulletin Solutions, Inc.
Theme made by Freecode