1
\$\begingroup\$

I have code that is used for retrieving and storing conversation for a chat application. I found this from a tutorial and tweak it a little bit. This script works fine but my concern is if it's the standard way. Can somebody please check this for me if to see if it's deprecated or a bad practice? And is there any other way of writing this script?

 //Gets the current messages from the server
            function getChatText() {
                if (receiveReq.readyState == 4 || receiveReq.readyState == 0) {
                    receiveReq.open("GET", 'includes/getChat.php?chat='+uid+'&last=' + lastMessage, true);
                    receiveReq.onreadystatechange = handleReceiveChat; 
                    receiveReq.send(null);
                }           
            }
            //Add a message to the chat server.
            function sendChatText() {

                if (sendReq.readyState == 4 || sendReq.readyState == 0) {
                    sendReq.open("POST", 'includes/getChat.php?last=' + lastMessage, true);
                    sendReq.setRequestHeader('Content-Type','application/x-www-form-urlencoded');
                    sendReq.onreadystatechange = handleSendChat; 
                    var param = 'message=' + document.getElementById('txtA').value;
                    param += '&name='+user;
                    param += '&uid='+uid;
                    param += '&rid='+document.getElementById('trg').value;
                    sendReq.send(param);
                    document.getElementById('txtA').value = '';
                }                           
            }
            //When our message has been sent, update our page.
            function handleSendChat() {
                //Clear out the existing timer so we don't have 
                //multiple timer instances running.
                clearInterval(mTimer);
                AjaxRetrieve();

            }
            function AjaxRetrieve()
            {
                var rid = document.getElementById('trg').value;
                $.get('includes/getChat.php?chat='+uid + '&rid=' + rid + '&name=' + user,function(data)
                {

                   $("#clog").html(data);



                });
            }
\$\endgroup\$
2
  • \$\begingroup\$ Can you show a complete example, including what receiveReq and sendReq are \$\endgroup\$
    – megawac
    Apr 3, 2014 at 4:28
  • \$\begingroup\$ @megawac I forgot to add these vars var sendReq = getXmlHttpRequestObject(); var receiveReq = getXmlHttpRequestObject(); sorry about that \$\endgroup\$ Apr 3, 2014 at 4:32

1 Answer 1

1
\$\begingroup\$

This is really odd. As I read the code, I was thinking you'd go all-native. Then, suddenly a $.get is found at the bottom. You had jQuery all along! Why not go ahead and use it for all the AJAX functions you have. Overhead is not really a concern here, but readability and maintainability is. Use jQuery all the way, keeps your code short.

// If they remain static, then move them out so you won't be running around the DOM
var message = $('#txtA').val();
var roomId = $('#trg').val();
var clog = $("#clog");

//Gets the current messages from the server
function getChatText() {
  $.get('includes/getChat.php', {
    chat: uid,
    last: lastMessage
  }, handleReceiveChat);
}

//Add a message to the chat server.
function sendChatText() {

  $.post('includes/getChat.php', {
    last: lastMessage,
    message: message,
    name: user,
    uid: uid,
    rid: roomId
  }, handleSendChat);
  message.val('');
}

function handleSendChat() {
  // No need to stop the timers. Have it run continuously.
}
// This function should retrieve the latest messages, and not everything.
// Also, if you are polling, use one timer that perpetually runs.
function AjaxRetrieve() {
  $.get('includes/getChat.php', {
    chat: uid,
    rid: roomId,
    name: user
  }, function (data) {
    clog.html(data);
  });
}

Other things I notice:

  • You manually pick out the values to send to the server. Why not place everything in a form, like place it in hidden input tags, and then take advantage of jQuery.serialize. This makes your code much easier by not doing document.getElement...value. You can hook up the form with a submit handler, prevent the normal submit, serialize the form and do ajax.

    <form id="chat">
      ...
      <input type="text" name="message">
      ...
    </form>
    
    <script>
      $('#chat').on('submit',function(event){
        // Prevent the normal submit
        event.preventDefault(); 
        // From here, it's as simple as a grab and go
        var formData = $(this).serialize();
        $.post('includes/getChat.php',formData,function(){...});
      });
    </script>
    
  • Polling is a good starter for doing semi-live chat. But that's stressful for the server. Imagine millions of people using your app, polling every 1 second for live data. That's 60 million requests to your server a minute! I suggest you look into WebSockets for a much more efficient implementation.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you for these wonderful pointers I'll keep this in mind.. Your right about the polling so I might really consider using WebSockets instead but I'm quite worried.. doesn't websockets have compatibility issues? \$\endgroup\$ Apr 3, 2014 at 5:56
  • 1
    \$\begingroup\$ @user256009 Here's the compatibility table, and you guessed it, IE. \$\endgroup\$
    – Joseph
    Apr 3, 2014 at 6:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.