12

There is multiple things that can be optimized in this code. It is pretty obvious that most time is spent in HTTP requests to remote API and in database queries, so these points should be first in list of optimizations. Your code doesn't tells much about what is coming from remote API and you didn't tell if you really need to run all requests for this API ...


5

This Function can be shorter. public function qdelete(){ $tbl = self::prepArgs(4, func_get_args()); $this->db->delete('Storage_Users'); if($this->db->affected_rows() > 0){ return TRUE; }else{ $this->msg = 'There was an issue removing that record.<br />' . $this->db->_error_message(); ...


4

Well, for a kickoff, you're concerned about your code's reusability can trump its undeniable downsides. But, if you look at one or two things more closely, your code isn't as reusable as perhaps you think it is: // Parsing arguments $params = (isset($args[0])) ? $args[0] : array(); $single = (isset($args[1])) ? $args[1] : false; $count = (isset($args[2])) ? ...


4

Formatting is not consistent: $this -> load -> model('user_login_model', 'login'); $user_id = $this->session->userdata('user_id'); Sometimes there are spaces around ->, sometimes aren't. You could use a guard clause to make the code flatten: $user_id = $this->session->userdata('user_id'); if ($user_id != TRUE) { return; } if (...


4

As Pimgd stated, all your request are synchronous (and wait that the previous one terminates before executing further). To make the calls asynchronous I see two possibilities: either the client (browser) does it: you send an almost empty page (with the token) and some Javascript code to perform the API calls (asynchronously). or the server does it and I ...


4

Looks pretty good. Here are just some thoughts but nothing major: It would be more future-proof (and probably efficient) to use regular expressions in lieu of parsing hard-coded line numbers (in case the data has rows added later): For the _save_data() you could parse like so: $string = '{THE CONTENT FROM THE COUNTRIES LIST PAGE}'; $sub = preg_replace('/.+?...


4

I had a very short look at lightblog/application/controllers/Pages.php and I noticed that these two values were not used in the page() method: $data['pages'] $data['posts'] Then I realized that you hand over data to other objects and they might be using these two values. Who knows? This makes your code very hard to understand and to debug. Basically you'...


4

There are a number of things I'd do differently and the first is a must. First, using md5() to encode a password is universally considered a bad practice. It's been considered too weak for that purpose since at least 2004 if not earlier. Instead, use the PHP function password_hash. I would encrypt the password in the model. In fact, I'd move almost all ...


3

Yes, you will miss every appointment that doesn't have :00 for seconds. And if your cron errs for any reason or skips a second, you'll miss even more. You should add another DATETIME column to your table called notified_at (or something like that) and then do two conditions in your where clause: WHERE start_datetime <= '$two_hour_time' AND notified_at IS ...


3

I would like to add something to @Dukeatcoding comment in the question. What is the point of passing the whole object? Thinking on it as a part of the main controller, you should only pass the data as an array to the model: $data = array( 'id_user' => $usuario, 'id_category' => $soporte->getCategoria 'message' => $soporte-&...


3

is this iteration pattern safe? Safe for what? Safe for a database? We can't tell you that! There's no code for your database entry function. What you have here is perfectly safe, but that's excluding any type of insert into a database. To safely protect yourself from SQL injection and other types of security exploits, use prepared queries, validate (...


3

This might be classified as a micro-optimization but... As handy as the Query Builder is, it runs quite a lot of code putting together a query statement. So instead of $this->db->select(...)->from(...)->where(...)->get(); Write out the query in a more direct manner. //First get incoming result and search in the database $sql = "SELECT ...


3

Validating emails is hard, and it's not a good idea to do it on your own with a regex. Here are a couple of valid email addresses you would not allow (there are a lot more): "foo"@bar.com, foo+spam@bar.com, !#$%&'*+-/=?^_{|}~@foo.com. And even though it's so strict, it still lets through invalid addresses such as foo@example.com] (because the regex is ...


3

No, it doesn't work like that because you're not escaping your PHP strings to be a safe JavaScript string. What you're doing is encoding them as JSON but if - for example - $res->token contains \ then it will produce a broken JavaScript string and JSON.stringify() won't fix it. Correct code must use json_encode() for this purpose. echo '<script>...


3

What's happening here is that if each request takes 0.3 seconds (pretty fast), the whole page takes 14 * 0.3 = 4.2 seconds to load (slow-ish). In cases like this, there's two things you could do. Run each request asynchronously. Alter the API to give you everything. By running the requests asynchronously, you're not waiting for the response of one request ...


3

Let's start with your controller (controllers/Volunteers.php): You have loaded list of models in __construct() which is not a good practice. Before loading list of models in __construct(), you must be sure, if you need methods from all the loaded models in most of the methods of your controllers. You loaded all the models without giving them a short object ...


3

Looking at your sql's you seem to be repeating the same queries in a loop. I will pick out one simple one for example, it doesn't take long to run on its own, but if you add them all up, the amount of roundtrips to the db grows. 0.0006 SELECT COUNT(*) AS `numrows` FROM `volunteer_shifts` WHERE `shift_id` = '82' AND `shift_deleted` =0 If you can get ...


3

Lots of little pieces of advice: Tab the lines of code inside of your loops and functions (etc) for improved readability. Only declare the $key variable in your foreach() loop if you intend to use it. Avoid performing iterated trips to the database. Furthermore, always endeavor to minimize total trips to the database unless you have a data volume issue to ...


3

In this case I would probably convert this $this->Static_model into a Config file that I can use through CI (that is what they are there for). You can even make CI load this file automatically, with hook. And you can even load different version based on your ENVIORMENT setting (for testing and what not). For the dynamic data, now depending how dynamic ...


3

Some food for thought: I don't see any benefit to str_shuffle() why bother? Why bother referencing the email? If you need a random string, populate one in the shortest way with satisfactory randomness for the task. I generally advise against any html markup being generated anywhere other than the view. I'd rather see your controller's properties contain ...


2

Is there any specific reason that you don't use Active Record? For example: $sth = $this->dbh->prepare("SELECT programs.id, programs.name FROM programs INNER JOIN program_users ON program_users.program_id = programs.id WHERE program_users....


2

What is the difference between this two way and Which one is best? What do you suggest? The difference is that you seperate the concerns in the second example. This means that you can change from database type (e.g. a SQL database, a XML file, an array, ect.) without changing to much code. In fact, you only create a new Model and you are ready! Whether ...


2

DRY The idea behind DRY is fairly deep: Every piece of knowledge must have a single, unambiguous, authoritative representation within a system. Avatar Developers often miss the idea that assigning a value to an instance variable in multiple places violates the DRY principle. Reading the code top-to-bottom reveals: $user_data->avatar = base_url() . $...


2

For your controller, a method like this would be best to call all your variables: $data = array( "add_user" => $this->load->view('form_add_user'), "username" => $username, "admin" => $admin ); (Variables are made up just for example) Then, within your code, do this: <div class="tab-pane" id="tab_1_2"> <div class=...


2

Apart from the fact I can't find any use of var loadPage It doesn't need to be an expression. Instead try: function loadPage(page) { } It is still accessible only within your code here, it is cleaner and easier to debug. Keep your click events consistent $(".sub-menu .menu-item, .module-item").click(function(event) { $(".page-content").load($(this)....


2

I haven't worked with CodeIgniter before, so if I say something that argues their standards, please excuse me. It's such a basic piece of code that there isn't too much to be said. One things I notice right off the bat, your checkbox_group() function has a first parameter with a default value. I'm surprised you haven't done anything about this because it ...


2

Well, having a parent model that is extended for all your model classes is always a good idea if they all share the same behaviour and help you with your development. I have implemented a similar approximation within my models and I'm doing something similar to what you do, but instead of writing the query in the controller, I call to the model to handle the ...


2

You need to extend CI_Controller with your code and extend your classses from your custom class, not default. I'll try to explain: At first create MY_Controller.php at /application/core/ with next content: <?php class MY_Controller extends CI_Controller { function __construct() { parent::__construct(); ...


2

You are right with your concerns regarding the model rendering the output. But you're almost there ain't you? Why don't you let your model return the data instead of the HTML-Code and let the controller call writeHtmlSearchPeople: class MSearchPeople extends CI_Model { function provideSearchPeopleResults($theCity) { $result = // db query, ...


Only top voted, non community-wiki answers of a minimum length are eligible