2
\$\begingroup\$

So I wrote this class to handle my ajax calls simply:

ajaxCon.php:

class ajaxCon{
 private $init = false;
 private $functionArray = array();
 private $Message = false;
 private $function = null;
 private $arguments = null;
 private $post = null;
 public function __construct(){
 if(!isset($_POST['ajaxRequest'], $_POST['ajaxFunction'])) return;
 $this->init = true;
 ob_start();
 $this->post = $this->sanitizeArr($_POST);
 $this->arguments = $this->sanitizeArr($_POST['arguments']);
 $this->function = $_POST['ajaxFunction'];
 }
 public function addFunction($functionName, $functionCode){
 if(!$this->init || !isset($functionName, $functionCode) || $functionName != $this->function) return $this;
 $this->Message = call_user_func_array($functionCode, $this->arguments) ? 'success' : 'fail' ;
 unset($this->init, $this->functionArray, $this->post);
 $returnVars = get_object_vars($this);
 ob_clean();
 echo json_encode($returnVars);
 ob_flush();
 exit();
 }
 private function sanitizeArr($returnArr, $sanitizer = 'sanitize_sql_string') {
 array_walk_recursive($returnArr, function (&$value) use ($sanitizer) {
 $value = $sanitizer($value);
 });
 return $returnArr;
 }
}

ajaxCon.js:

function ajax() {
 this.callback = false;
 this.ajaxFailure = function (returnData) {};
 this.ajaxSuccess = function (returnData) {};
 this.call = function(functionName, argumentArray, callback){
 $.ajax( $.extend(true, {}, this.ajaxSettings, {data:{'ajaxFunction':functionName, 'arguments':argumentArray}} ) );
 this.callback = callback;
 };
 this.ajaxReturn = function (returnData){
 returnData = returnData.substring(returnData.indexOf("{") - 1);
 returnData = returnData.substring(0, returnData.lastIndexOf("}") + 1);
 var responseArray = $.parseJSON(returnData);
 if (responseArray.Message.toLowerCase() != "success"){
 this.ajaxFailure(responseArray);
 return;
 }
 this.ajaxSuccess(responseArray);
 if(this.callback != false) this.callback(responseArray);
 };
 window.ajaxCon = this;
 this.ajaxSettings = { url: window.location.href, type: 'POST', cache: false, data: {ajaxRequest: true}, success: function (returnData) {
 ajaxCon.ajaxReturn(returnData)
 }};
}

and you call it like so:

<?php
 $ajaxCon = new ajaxCon;
 $ajaxCon->addFunction('checkEmail', function($Email){
 global $ajaxCon;
 if(emailIsInDB($Email)) $ajaxCon->hasAccount = true;
 return true;
 });
?>
<script type="text/javascript">
 var ajaxCon = new ajaxCon();
 function checkEmail() {
 ajaxCon.call('checkEmail', {'Email':$('#Email').val()}, function(result){
 if(!result.hasAccount) $('#SignUp').submit();
 });
 }
</script>

The drawbacks I know of are lot's of preprocessing (The entire page has to be preprocesed) and the page then executes as normal until the php function I need is defined and called.

Any ideas on how I can optimize the code and make it run more efficiently while preserving/improving readability?

asked May 28, 2014 at 17:45
\$\endgroup\$
1
  • \$\begingroup\$ Take a look at how swagger does it. It's basically a contract for publishing metadata about restful services, sort of like SOAP's WSDL. Might give you some ideas (or you could just go with something like restler php + swagger js client and scrap this code). \$\endgroup\$ Commented May 28, 2014 at 20:42

2 Answers 2

4
\$\begingroup\$

A review of the JavaScript part:

  • ajaxCon is a class/constructor, so you should call it AjaxCon, though really I would think about finding an even better name :)
  • ajaxFailure -> You have a number of functions that have ajax in their name, since these functions are in the Ajax constructor you can drop the ajax part
  • this.callback = false; -> Don't declare callback at all and then use
    if(this.callback) this.callback(responseArray);
  • This could use some newlines:
    $.ajax( $.extend(true, {}, this.ajaxSettings, {data:{'ajaxFunction':functionName, 'arguments':argumentArray}} ) );

    and also

    this.ajaxSettings = { url: window.location.href, type: 'POST', cache: false, data: {ajaxRequest: true}, success: function (returnData) {
     ajaxCon.ajaxReturn(returnData)
    }};
    
  • Also the location of this.ajaxSettings is odd, this belongs on top of that function
answered May 28, 2014 at 22:41
\$\endgroup\$
1
  • 2
    \$\begingroup\$ I'd mention that .call is a poor method name choice in javascript at first glance it meant Function.prototype.call \$\endgroup\$ Commented May 28, 2014 at 23:29
2
\$\begingroup\$

Review for the PHP part:

  • What problem does your class solve? Are you not doing too many things in one class? If your goal is to handle incoming Ajax requests, then a different object should handle the response.

  • Class names should be in CamelCaps, i.e. AjaxCon.

  • If you need something in your constructor (data from POST, etc), just ask for it in the constructor:

     public function __construct(array $post) {
    
  • Don't try to sanitize the entire $_POST array at once, it never works. What are you sanitizing it for? Data that goes into the database should go through prepared statements , data that gets outputted back to the page as HTML should be run through htmlspecialchars() before they are outputted to the page. Don't sanitize data until the moment you need it, otherwise you risk double sanitation problems (Tom & Jerry becomes Tom &amp; Jerry which becomes Tom &amp;amp; Jerry, oops).

  • Don't output in your function. Always return. Output should be on a single place only, and that's at the very end of the run, where you need to return the response after all processing is done.

  • Don't use get_object_vars($this) for the purpose you are using it. Specify exactly what variables you are returning, it's more readable.

  • Don't use globals - If you need to pass a variable from the outside to an anonymous function, use use:

     function($email) use ($ajaxCon) {
    
  • DO NOT USE GLOBALS - Seriously, don't.

answered May 29, 2014 at 6:55
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.