feat:improve code quality

casbin/config/config.cpp

@@ -25,6 +25,7 @@
 #include <algorithm>
 #include <cstdio>
 
+
 #include "./config.h"
 #include "../exception/io_exception.h"
 #include "../exception/illegal_argument_exception.h"
@@ -38,20 +39,20 @@ mutex Config::mtx_lock;
 /**
  * addConfig adds a new section->key:value to the configuration.
  */
-bool Config :: AddConfig(string section, string option, string value) {
+bool Config :: AddConfig(string section, string& option, string& value) {
     if (!section.compare(""))
         section = DEFAULT_SECTION;
     bool ok = data[section].find(option) != data[section].end();
     data[section][option] = value;
     return !ok;
 }
 
-void Config :: Parse(string f_name) {
+void Config :: Parse(string& f_name) {
     mtx_lock.lock();
     ifstream infile;
     try {
         infile.open(f_name);
-    } catch (const ifstream::failure e) {
+    } catch (const ifstream::failure& e) {
         mtx_lock.unlock();
         throw IOException("Cannot open file.");
     }
@@ -73,18 +74,19 @@ void Config :: ParseBuffer(istream* buf){
         else
             break;
         line = Trim(line);
-        if (line.find(DEFAULT_COMMENT)==0)
+        if (line.compare(DEFAULT_COMMENT)==0)
             continue;
-        else if (line.find(DEFAULT_COMMENT_SEM)==0)
+        else if (line.compare(DEFAULT_COMMENT_SEM)==0)
             continue;
-        else if (line.find("[")==0 && EndsWith(line, string("]")))
+        else if (line.compare("[")==0 && EndsWith(line, string("]")))
             section = line.substr(1, line.length() - 2);
         else {
-            vector<string> option_val = Split(line, string("="), 2);
+            vector<string> option_val = Split(line, string("="));
             if (option_val.size() != 2) {
-                char* error = new char;
-                sprintf(error, "parse the content error : line %d , %s = ? ", line_num, option_val[0].c_str());
-                throw IllegalArgumentException(string(error));
+                //char* error = new char;
+                string error;
+                sprintf(const_cast<char*>( error.data() ), "parse the content error : line %d , %s = ? ", line_num, option_val[0].c_str());
+                throw IllegalArgumentException(error);
             }
             string option = Trim(option_val[0]);
             string value = Trim(option_val[1]);
@@ -111,30 +113,31 @@ shared_ptr<Config> Config :: NewConfig(string conf_name) {
  * @param text the model text.
  * @return the constructor of Config.
  */
-shared_ptr<Config> Config :: NewConfigFromText(string text) {
+shared_ptr<Config> Config :: NewConfigFromText(string& text) {
     shared_ptr<Config> c(new Config);
     stringstream stream(text);
     c->ParseBuffer(&stream);
     return c;
 }
 
-bool Config :: GetBool(string key) {
+bool Config :: GetBool(string& key) {
     return Get(key).compare("true")==0;
 }
 
-int Config :: GetInt(string key) {
+int Config :: GetInt(string& key) {
     return atoi(Get(key).c_str());
 }
 
-float Config :: GetFloat(string key) {
-    return float(atof(Get(key).c_str()));
+float Config :: GetFloat(string& key) {
+    return static_cast<float >( atof(Get(key).c_str()) );
 }
 
-string Config :: GetString(string key) {
+
+string Config :: GetString(const string& key) {
     return Get(key);
 }
 
-vector<string> Config :: GetStrings(string key) {
+vector<string> Config :: GetStrings(string& key) {
     string v = Get(key);
     if (!v.compare("")) {
         vector<string> empty;
@@ -143,7 +146,7 @@ vector<string> Config :: GetStrings(string key) {
     return Split(v,string(","));
 }
 
-void Config :: Set(string key, string value) {
+void Config :: Set(string key, string& value) {
     mtx_lock.lock();
     if (key.length() == 0) {
         mtx_lock.unlock();
@@ -166,6 +169,7 @@ void Config :: Set(string key, string value) {
     mtx_lock.unlock();
 }
 
+
 string Config :: Get(string key) {
     string section;
     string option;

casbin/config/config.h

@@ -20,6 +20,7 @@
 #include <memory>
 #include <unordered_map>
 #include <mutex>
+#include <filesystem>
 
 #include "./config_interface.h"
 
@@ -38,14 +39,14 @@ class Config : public ConfigInterface {
         /**
          * addConfig adds a new section->key:value to the configuration.
          */
-        bool AddConfig(string section, string option, string value);
+        bool AddConfig(string section, string& option, string& value);
 
-        void Parse(string f_name);
+        void Parse(string& f_name);
 
         void ParseBuffer(istream* buf);
 
     public:
-
+        ~Config()=default;
         /**
          * NewConfig create an empty configuration representation from file.
          *
@@ -60,21 +61,21 @@ class Config : public ConfigInterface {
          * @param text the model text.
          * @return the constructor of Config.
          */
-        static shared_ptr<Config> NewConfigFromText(string text);
+        static shared_ptr<Config> NewConfigFromText(string& text);
 
-        bool GetBool(string key);
+        bool GetBool(string& key);
 
-        int GetInt(string key);
+        int GetInt(string& key);
 
-        float GetFloat(string key);
+        float GetFloat(string& key);
 
-        string GetString(string key);
+        string GetString(const string& key);
 
-        vector<string> GetStrings(string key);
+        vector<string> GetStrings(string& key);
 
-        void Set(string key, string value);
+        void Set(string key, string& value);
 
-        string Get(string key);
+        string Get(string k);
 };
 
 #endif

casbin/config/config_interface.h

@@ -24,14 +24,14 @@ using namespace std;
 
 class ConfigInterface {
     public:
+        virtual ~ConfigInterface()=0;
 
-        virtual string GetString(string key) = 0;
-        virtual vector <string> GetStrings(string key) = 0;
-        virtual bool GetBool(string key) = 0;
-        virtual int GetInt(string key) = 0;
-        virtual float GetFloat(string key) = 0;
-        virtual void Set(string key, string value) = 0;
-
+        virtual string GetString(const string& key) = 0;
+        virtual vector <string> GetStrings(string& key) = 0;
+        virtual bool GetBool(string& key) = 0;
+        virtual int GetInt(string& key) = 0;
+        virtual float GetFloat(string& key) = 0;
+        virtual void Set(string key, string& value) = 0;
 };
 
 #endif

casbin/effect/default_effector.cpp

@@ -26,28 +26,28 @@
 /**
  * MergeEffects merges all matching results collected by the enforcer into a single decision.
  */
-bool DefaultEffector :: MergeEffects(string expr, vector<Effect> effects, vector<float> results) {
+bool DefaultEffector :: MergeEffects(string expr, vector<Effect>& effects, vector<float>& results) {
     bool result;
 
     if (!expr.compare("some(where (p.eft == allow))")) {
         result = false;
-        for(unsigned int index = 0 ; index < effects.size() ; index++){
+        for(auto index = 0 ; index < effects.size() ; index++){
             if (effects[index] == Effect::Allow) {
                 result = true;
                 break;
             }
         }
     } else if (!expr.compare("!some(where (p.eft == deny))")) {
         result = true;
-        for(unsigned int index = 0 ; index < effects.size(); index++){
+        for(auto index = 0 ; index < effects.size(); index++){
             if (effects[index] == Effect::Deny) {
                 result = false;
                 break;
             }
         }
     } else if (!expr.compare("some(where (p.eft == allow)) && !some(where (p.eft == deny))")) {
         result = false;
-        for(unsigned int index = 0 ; index < effects.size(); index++){
+        for(auto index = 0 ; index < effects.size(); index++){
             if (effects[index] == Effect::Allow) {
                 result = true;
             } else if (effects[index] == Effect::Deny) {
@@ -57,7 +57,7 @@ bool DefaultEffector :: MergeEffects(string expr, vector<Effect> effects, vector
         }
     } else if (!expr.compare("priority(p.eft) || deny")) {
         result = false;
-        for(unsigned int index = 0 ; index < effects.size(); index++){
+        for(auto index = 0 ; index < effects.size(); index++){
             if (effects[index] != Effect::Indeterminate) {
                 if (effects[index] == Effect::Allow) {
                     result = true;

casbin/effect/default_effector.h

@@ -24,11 +24,11 @@
  */
 class DefaultEffector : public Effector{
     public:
-
+        ~DefaultEffector()=default;
         /**
          * MergeEffects merges all matching results collected by the enforcer into a single decision.
          */
-        bool MergeEffects(string expr, vector<Effect> effects, vector<float> results);
+        bool MergeEffects(string expr, vector<Effect>& effects, vector<float>& results);
 };
 
 #endif

casbin/effect/effector.h

@@ -37,7 +37,8 @@ class Effector{
          * @param results the matcher results of all matched rules.
          * @return the final effect.
          */
-        virtual bool MergeEffects(string expr, vector<Effect> effects, vector<float> results) = 0;
+        virtual bool MergeEffects(string expr, vector<Effect>& effects, vector<float>& results) = 0;
+        virtual ~Effector()=0;
 };
 
 #endif