From ebb69b60969d33fa2f73184cc50db1f8a54adc0d Mon Sep 17 00:00:00 2001 From: VaclavT Date: Sun, 2 Jan 2022 12:45:53 +0100 Subject: [PATCH] some refactorings --- doc/Doc.md | 2 +- ml.cpp | 77 ++++++++++++++++++++++-------------------- ml.h | 8 ++--- ml_string.cpp | 4 +-- ml_string.h | 2 +- ml_util.cpp | 9 +++-- usql/settings.cpp | 2 +- usql/table.cpp | 2 +- usql/usql.cpp | 2 +- usql/usql_function.cpp | 3 +- 10 files changed, 57 insertions(+), 54 deletions(-) diff --git a/doc/Doc.md b/doc/Doc.md index 55b4049..3446062 100644 --- a/doc/Doc.md +++ b/doc/Doc.md @@ -53,7 +53,7 @@ |`(% a b)`|Remainder of division|result of operation| |`(list ..)`|Create a list of values|| |`(insert list index element)`|Insert an element into a list. Indexed from 0|new list with value inserted| -|`(index list index)`|Return element at index in list|Element at index| +|`(index list index)`|Return element at index in list. First element is at index 0|Element at index| |`(remove list index)`|Remove a value at an index from a list|List with element removed| |`(len list)`|Get the length of a list|list length| |`(push list element)`|Add an item to the end of a list|new list with element added| diff --git a/ml.cpp b/ml.cpp index 0362f7e..599d6d5 100644 --- a/ml.cpp +++ b/ml.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -81,7 +82,7 @@ MlValue::MlValue(bool b) { else type = NIL; } -MlValue::MlValue(const std::vector &list) : type(LIST), list(list) {} +MlValue::MlValue(std::vector list) : type(LIST), list(std::move(list)) {} MlValue::MlValue(const std::vector &slist) : type(LIST) { list.reserve(slist.size()); @@ -618,7 +619,7 @@ std::ostream &operator<<(std::ostream &os, MlValue const &v) { } -MlError::MlError(const MlValue &v, MlEnvironment const &env, const char *msg) : env(env), msg(msg) { +MlError::MlError(const MlValue &v, const MlEnvironment &env, const char *msg) : env(env), msg(msg) { cause = new MlValue; *cause = v; } @@ -660,11 +661,11 @@ std::ostream &operator<<(std::ostream &os, MlEnvironment const &e) { } void MlEnvironment::set(const std::string &name, MlValue value) { - defs[name] = value; + defs[name] = std::move(value); } -void MlEnvironment::setX(const std::string &name, MlValue value) { +void MlEnvironment::setX(const std::string &name, const MlValue& value) { MlEnvironment *e = this; while (e != nullptr) { auto itr = e->defs.find(name); @@ -729,7 +730,7 @@ MlValue MlValue::eval(MlEnvironment &env) { case ATOM: return env.get(str); case LIST: - if (list.size() < 1) + if (list.empty()) return MlValue::nil(); args = std::vector(list.begin() + 1, list.end()); @@ -740,8 +741,8 @@ MlValue MlValue::eval(MlEnvironment &env) { // Builtin functions can be special forms, so we // leave them to evaluate their arguments. if (!function.is_builtin()) - for (size_t i = 0; i < args.size(); i++) - args[i] = args[i].eval(env); + for (auto & arg : args) + arg = arg.eval(env); MlPerfMon::instance().add_method_call(function, args); res = function.apply(args, env); @@ -781,7 +782,7 @@ MlValue parse(std::string &s, int &ptr) { throw std::runtime_error(INTERNAL_ERROR); - if (s == "") { + if (s.empty()) { return MlValue(); } else if (s[ptr] == '\'') { @@ -812,7 +813,7 @@ MlValue parse(std::string &s, int &ptr) { skip_whitespace(s, ptr); if (n.find('.') != std::string::npos) - return MlValue((negate ? -1l : 1l) * atof(n.c_str())); + return MlValue((negate ? -1.0 : 1.0) * atof(n.c_str())); else return MlValue((negate ? -1l : 1l) * atol(n.c_str())); } else if (s[ptr] == '\"') { @@ -1049,8 +1050,11 @@ MlValue scope(std::vector args, MlEnvironment &env) { // Quote an expression (SPECIAL FORM) MlValue quote(std::vector args, MlEnvironment &env) { std::vector v; + + v.reserve(args.size()); for (const auto &arg : args) v.push_back(arg); + return MlValue(v); } @@ -1085,7 +1089,7 @@ MlValue exit(std::vector args, MlEnvironment &env) { for(auto &t : threads_register) t.detach(); - std::exit(args.size() < 1 ? 0 : args[0].cast_to_int().as_int()); + std::exit(args.empty() ? 0 : (int)args[0].cast_to_int().as_int()); return MlValue(); // will not be called :-) } @@ -1093,7 +1097,7 @@ MlValue exit(std::vector args, MlEnvironment &env) { MlValue print(std::vector args, MlEnvironment &env) { eval_args(args, env); - if (args.size() < 1) + if (args.empty()) throw MlError(MlValue("print", print), env, TOO_FEW_ARGS); MlValue acc; @@ -1194,8 +1198,8 @@ MlValue read_url(std::vector args, MlEnvironment &env) { eval_args(args, env); // PERF optimize it for memory usage and performance - if (args.size() < 1 || args.size() > 2) - throw MlError(MlValue("read_url", read_url), env, args.size() < 1 ? TOO_FEW_ARGS : TOO_MANY_ARGS); + if (args.empty() || args.size() > 2) + throw MlError(MlValue("read_url", read_url), env, args.empty() ? TOO_FEW_ARGS : TOO_MANY_ARGS); std::unordered_map headers = {}; HttpClient client; @@ -1237,7 +1241,7 @@ MlValue parse_json(std::vector args, MlEnvironment &env) { MlValue get_universal_time(std::vector args, MlEnvironment &env) { eval_args(args, env); - if (args.size() != 0) + if (!args.empty()) throw MlError(MlValue("get-universal-time", get_universal_time), env, TOO_MANY_ARGS); return MlValue(now()); @@ -1247,7 +1251,7 @@ MlValue get_universal_time(std::vector args, MlEnvironment &env) { MlValue get_localtime_offset(std::vector args, MlEnvironment &env) { eval_args(args, env); - if (args.size() != 0) + if (!args.empty()) throw MlError(MlValue("get-localtime-offset", get_localtime_offset), env, TOO_MANY_ARGS); return MlValue(get_gmt_localtime_offset()); @@ -1280,7 +1284,7 @@ MlValue date_add(std::vector args, MlEnvironment &env) { if (args.size() != 3) throw MlError(MlValue("date-add", date_add), env, args.size() > 3 ? TOO_MANY_ARGS : TOO_FEW_ARGS); - return MlValue(add_to_date(args[0].as_int(), args[1].as_int(), args[2].as_string())); + return MlValue(add_to_date(args[0].as_int(), (int)args[1].as_int(), args[2].as_string())); } @@ -1345,7 +1349,7 @@ MlValue tcp_server(std::vector args, MlEnvironment &env) { }; TcpNet server; - int r = server.server(args[0].as_int(), proccess_req); + int r = server.server((int)args[0].as_int(), proccess_req); return MlValue((long)r); } @@ -1361,10 +1365,10 @@ MlValue tcp_client(std::vector args, MlEnvironment &env) { std::vector requests;// PERF reserve std::transform(request_list.begin(), request_list.end(), back_inserter(requests), std::mem_fn(&MlValue::as_string)); - std::vector response = tcpclient.client(args[0].as_string(), args[1].as_int(), requests); + std::vector response = tcpclient.client(args[0].as_string(), (int)args[1].as_int(), requests); return MlValue(response); } else { - std::string response = tcpclient.client(args[0].as_string(), args[1].as_int(), args[2].as_string()); + std::string response = tcpclient.client(args[0].as_string(), (int)args[1].as_int(), args[2].as_string()); return MlValue::string(response); } } @@ -1623,7 +1627,7 @@ MlValue len(std::vector args, MlEnvironment &env) { MlValue push(std::vector args, MlEnvironment &env) { eval_args(args, env); - if (args.size() == 0) + if (args.empty()) throw MlError(MlValue("push", push), env, TOO_FEW_ARGS); for (size_t i = 1; i < args.size(); i++) @@ -1739,8 +1743,9 @@ MlValue string_regex_list(std::vector args, MlEnvironment &env) { auto found_matches = regexp_search2(args[0].as_string(), args[1].as_string(), match_mode, ignore_case); std::vector list; + list.reserve(found_matches.size()); for(auto &item : found_matches) { - list.push_back(item); + list.emplace_back(item); } return MlValue(list); } @@ -1752,7 +1757,6 @@ MlValue string_split(std::vector args, MlEnvironment &env) { if (args.size() != 2) throw MlError(MlValue("string-split", string_split), env, args.size() > 2 ? TOO_MANY_ARGS : TOO_FEW_ARGS); - // TODO do it more efficient std::vector elements = regexp_strsplit(args[0].as_string(), args[1].as_string()); return MlValue(elements); } @@ -1779,12 +1783,12 @@ MlValue string_len(std::vector args, MlEnvironment &env) { MlValue string_substr(std::vector args, MlEnvironment &env) { eval_args(args, env); - if (args.size() < 1 || args.size() > 3) + if (args.empty() || args.size() > 3) throw MlError(MlValue("string-substr", string_substr), env, args.size() > 3 ? TOO_MANY_ARGS : TOO_FEW_ARGS); const std::string &str = args[0].as_string(); long pos = args.size() > 1 ? args[1].as_int() : 0; - long count = args.size() > 2 ? args[2].as_int() : str.size(); + auto count = args.size() > 2 ? args[2].as_int() : str.size(); return MlValue::string(string_substring(str, pos, count)); } @@ -1819,8 +1823,7 @@ MlValue string_pad(std::vector args, MlEnvironment &env) { if (args.size() != 4) throw MlError(MlValue("string_pad", string_pad), env, args.size() > 4 ? TOO_MANY_ARGS : TOO_FEW_ARGS); - // TODO validate len > 0 etc - return MlValue::string(string_padd(args[0].as_string(), args[1].as_int(), args[2].as_string()[0], + return MlValue::string(string_padd(args[0].as_string(), (size_t)args[1].as_int(), args[2].as_string()[0], (args[3].as_string() == "rpad"))); } @@ -1845,7 +1848,7 @@ MlValue debug(std::vector args, MlEnvironment &env) { MlValue sprintf(std::vector args, MlEnvironment &env) { eval_args(args, env); - if (args.size() < 1 || args.size() > 2) + if (args.empty() || args.size() > 2) throw MlError(MlValue("sprintf", sprintf), env, args.size() > 2 ? TOO_MANY_ARGS : TOO_FEW_ARGS); return MlValue::string(mini_sprintf(args[0].as_string(), args.size() == 2 ? args[1].as_list() : std::vector{})); @@ -1969,7 +1972,7 @@ MlValue thread_create(std::vector args, MlEnvironment &env) { static_assert(sizeof(std::thread::id) == sizeof(uint64_t), "size of thead::id is not equal to the size of uint_64"); - uint64_t *ptr = (uint64_t *) &th_id; + auto *ptr = (uint64_t *) &th_id; long tid = (*ptr); return MlValue(tid); @@ -2001,7 +2004,7 @@ MlValue thread_sleep(std::vector args, MlEnvironment &env) { } MlValue threads_join(std::vector args, MlEnvironment &env) { - if (args.size() != 0) + if (!args.empty()) throw MlError(MlValue("threads-join", threads_join), env, TOO_MANY_ARGS); // here is a question about using lockGuard, when used it holds lockGuard locked until @@ -2076,12 +2079,13 @@ bool MlEnvironment::has(const std::string &name) const { -std::map builtin_funcs { +std::map builtin_funcs +{ // Special forms std::make_pair("define", builtin::define), std::make_pair("lambda", builtin::lambda), std::make_pair("if", builtin::if_then_else), - std::make_pair("if", builtin::cond), + std::make_pair("cond", builtin::cond), std::make_pair("do", builtin::do_block), std::make_pair("for", builtin::for_loop), std::make_pair("while", builtin::while_loop), @@ -2194,7 +2198,7 @@ std::map builtin_funcs { // Exceptions std::make_pair("try", builtin::try_block), std::make_pair("throw", builtin::throw_exception), - + // Usql std::make_pair("usql", builtin::usql) }; @@ -2202,7 +2206,6 @@ std::map builtin_funcs { // Get the value associated with this name in this scope MlValue MlEnvironment::get(const std::string &name) const { - // PERF, here can be a few of for fast access if (name == "define") return MlValue("define", builtin::define); if (name == "if") return MlValue("if", builtin::if_then_else); @@ -2262,7 +2265,7 @@ void repl(MlEnvironment &env) { std::getline(std::cin, input); write_file_contents(input, code); - } else if (input != "") { + } else if (!input.empty()) { try { tmp = run(input, env); std::cout << " => " << tmp.debug() << std::endl; @@ -2289,7 +2292,7 @@ std::vector getCmdOption(char *argv[], int argc, const std::string for (int i = 1; i < argc; ++i) { if (option == argv[i] && i + 1 < argc) { i++; - tokens.push_back(std::string(argv[i])); + tokens.emplace_back(argv[i]); } } return tokens; @@ -2298,6 +2301,8 @@ std::vector getCmdOption(char *argv[], int argc, const std::string int main(int argc, char *argv[]) { MlEnvironment env; std::vector args; + + args.reserve(argc); for (int i = 0; i < argc; i++) args.push_back(MlValue::string(argv[i])); env.set("cmd-args", MlValue(args)); @@ -2338,7 +2343,7 @@ int main(int argc, char *argv[]) { for (auto & file : getCmdOption(argv, argc, "-run")) { // TODO check only one file is specified ?? std::string file_content = read_file_contents(file); if (file_content.find("#!") == 0) // shebang ? - file_content.erase(0, file_content.find("\n") + 1); // TODO mac osx newline?? + file_content.erase(0, file_content.find('\n') + 1); // TODO mac osx newline?? run(file_content, env); } diff --git a/ml.h b/ml.h index d6d2e84..63c4a9c 100644 --- a/ml.h +++ b/ml.h @@ -43,7 +43,7 @@ public: // Set the value associated with this name in this scope and parent scopes // and if not exists sets in this scope - void setX(const std::string &name, MlValue value); + void setX(const std::string &name, const MlValue& value); // Get vector of executables in this scope std::vector get_lambdas_list() const; @@ -71,7 +71,7 @@ public: // Create an error with the value that caused the error, // the scope where the error was found, and the message. - MlError(const MlValue &v, MlEnvironment const &env, const char *msg); + MlError(const MlValue &v, const MlEnvironment &env, const char *msg); // Copy constructor is needed to prevent double frees MlError(MlError const &other); @@ -104,7 +104,7 @@ public: MlValue(long i); MlValue(double f); MlValue(bool b); - MlValue(const std::vector &list); // Constructs a list + MlValue(std::vector list); // Constructs a list MlValue(const std::vector &slist); // Constructs a list from vector of strings static MlValue quote(const MlValue "ed); // Construct a quoted value @@ -136,7 +136,7 @@ public: std::vector as_list() const; // Push an item to the end of this list - void push(MlValue val); + void push(const MlValue& val); // Push an item from the end of this list MlValue pop(); diff --git a/ml_string.cpp b/ml_string.cpp index 5473a2c..e637dba 100644 --- a/ml_string.cpp +++ b/ml_string.cpp @@ -134,7 +134,7 @@ std::string string_padd(const std::string &str, size_t pad_len, char fill_char, } -std::string string_substring(const std::string & str, long pos, long count) { +std::string string_substring(const std::string & str, long pos, size_t count) { size_t start_pos = pos; if (pos < 0) { @@ -155,5 +155,5 @@ size_t string_find_substr(const std::string & str, const std::string & pattern, size_t p = str.find(pattern, pos); - return p != str.npos ? p : -1; + return p != std::string::npos ? p : -1; } diff --git a/ml_string.h b/ml_string.h index 0a25e42..3b0d941 100644 --- a/ml_string.h +++ b/ml_string.h @@ -20,6 +20,6 @@ std::string string_trim(std::string s, const std::string &chars_to_trim, const s std::string string_padd(const std::string & str, size_t pad_len, char fill_char, bool from_right); -std::string string_substring(const std::string & str, long pos, long count); +std::string string_substring(const std::string & str, long pos, size_t count); size_t string_find_substr(const std::string & str, const std::string & pattern, size_t pos); diff --git a/ml_util.cpp b/ml_util.cpp index cbdbf32..fb74d01 100644 --- a/ml_util.cpp +++ b/ml_util.cpp @@ -25,7 +25,6 @@ std::string get_history_file_dir() { else return std::string{t} + "/" + file; } -// TODO fujtajbl MlEnvironment * repl_env = nullptr; void setup_linenoise(const MlEnvironment &env) { @@ -69,13 +68,13 @@ void completion(const char *buf, linenoiseCompletions *lc) { std::string token = str.substr(pos+1); std::string begining = str.substr(0, pos+1); - // TODO optimize not to get all lambdas, but those begining with token + // TODO optimize not to get all lambdas, but those beginning with token std::vector lambdas = repl_env->get_lambdas_list(); lambdas.insert(end(lambdas), begin(commands), end(commands)); - for (std::vector::iterator t = lambdas.begin(); t != lambdas.end(); ++t) { - if(t->find(token) == 0) { - std::string completion_string = begining + *t; + for (const auto & lambda : lambdas) { + if(lambda.find(token) == 0) { + std::string completion_string = begining + lambda; linenoiseAddCompletion(lc, completion_string.c_str()); } } diff --git a/usql/settings.cpp b/usql/settings.cpp index ca36be8..f884af9 100644 --- a/usql/settings.cpp +++ b/usql/settings.cpp @@ -19,7 +19,7 @@ std::vector> Settings::m_settings = long Settings::string_to_long(const std::string &intstr) { try { return std::stol(intstr); - } catch (std::invalid_argument &e) { + } catch (const std::invalid_argument &e) { throw Exception("error parsing as integer: " + intstr); } } diff --git a/usql/table.cpp b/usql/table.cpp index 89e7128..f04e18b 100644 --- a/usql/table.cpp +++ b/usql/table.cpp @@ -183,7 +183,7 @@ void Table::commit_row(Row &row) { try { validate_row(row); index_row(row); - } catch (Exception &e) { + } catch (const Exception &e) { throw e; } } diff --git a/usql/usql.cpp b/usql/usql.cpp index dc901fe..3835007 100644 --- a/usql/usql.cpp +++ b/usql/usql.cpp @@ -11,7 +11,7 @@ std::unique_ptr USql::execute(const std::string &command) { std::unique_ptr node = m_parser.parse(command); return execute(*node); - } catch (std::exception &e) { + } catch (const std::exception &e) { return create_stmt_result_table(-1, e.what(), 0); } diff --git a/usql/usql_function.cpp b/usql/usql_function.cpp index dba9ccc..deef75c 100644 --- a/usql/usql_function.cpp +++ b/usql/usql_function.cpp @@ -82,8 +82,7 @@ std::unique_ptr USql::pp_function(const std::vector(parsed_value->getStringValue()); } -std::unique_ptr -USql::max_function(const std::vector> &evaluatedPars, const ColDefNode *col_def_node, ColValue *agg_func_value) { +std::unique_ptr USql::max_function(const std::vector> &evaluatedPars, const ColDefNode *col_def_node, ColValue *agg_func_value) { if (col_def_node->type == ColumnType::integer_type || col_def_node->type == ColumnType::date_type) { if (!evaluatedPars[0]->isNull()) { auto val = evaluatedPars[0]->getIntegerValue();