PHP Classes

Some suggestions

Recommend this page to a friend!

      AJAX driven simple chat  >  AJAX driven simple chat package blog  >  PHP Chat System 2020 ...  >  All threads  >  Some suggestions  >  (Un) Subscribe thread alerts  
Subject:Some suggestions
Summary:Since you asked for ideas to improve your code...
Messages:2
Author:Bodor Bence
Date:2015-06-29 19:09:49
 

  1. Some suggestions   Reply   Report abuse  
Picture of Bodor Bence Bodor Bence - 2015-06-29 19:09:49
Hi there.

Nice article. =)

First comment of mine, btw. Thought I would drop a few suggestions, since you asked for it at the end.

First of all, I noticed you inconsistency of giving variable names or ids. Sometimes you use camel case and sometimes underscores. This is confusing and a bit hard to read like that. Decide which style you want to use and keep that through all of your code. Much easier to read and maintain.
And that wasn`t the first thing that poke into my eye... I noticed you wrote "usrname" instead of "username" as the nickname field in the mysql table. At first I thought it was a simple typo, but then I saw that you use it like that everywhere... I don`t know about you, but for me, it is harder to write that down then the actual word and you only spared one character by it. Not really worth it if you ask me. Ok, that`s just me being a bit picky. Sorry.
And one last picky part... In PHP constants are usually written in uppercase cause that way you are sure it is a constant and not a varible missing a dollar sign by accident or something. It`s just an easy way to spot the constants right when you see it without further thinking.

Now for the actual code. I will only go into details for the PHP part now.
Since you wrote a class, I think you should use the advantages of using one. The class properties for example. You opened a connection in both of the classes methods. Why not write a method for the database connection and use that in both methods? Also, if you put the connection in its own method, you can store the connection resource in a class property so you can reuse it if you decide to add more methods which would need to use a database connection in the class later.
The other thing is you use of the json_encode function... You should only use that once in the return of the function. Instead, yo use it in a foreach for every single row. That is excessive and unnecessary. Not optimized. You should build the return as an array only, and then when you populated the whole array, use return json_encode($jsonData); as your return line.
Also, you don`t seem to give any notification about the status of the saving of a new row. What would happen if something goes wrong during saving? You should notify the user in that case.
And one more thing that you should really avoid... Depending on the timezone of the database. Never do that. If you decide to migrate your code and database to a different server than the developement one, you might get funny dates and times cause there is a possibility that the new server has a different time set as default. You have more control over it in the PHP environment anyway and an easy way to keep your times the same on any server after any kind of migration.

Those are my few suggestions for now.

And maybe you could try to add a feature where two people can chat privately too. I bet a lot of people woul like that. =)

  2. Re: Some suggestions   Reply   Report abuse  
Picture of Ashraf Gheith Ashraf Gheith - 2015-06-29 22:07:57 - In reply to message 1 from Bodor Bence
Thank you Bodor, this was very helpful. I will definitely take care about this in the future. I did not focus a lot on the code because I spent a lot of energy writing the article. So that is the mistake. When I look at the code now, I can see a lot of things I could do better, so a new version is a must :)

About the DB method, it is a very good idea, did not think of that before. I have a class I wrote at work with at least 50 methods, and every one makes a connection, so that would optimize the code really.

Also the json thing, I will surly use that, I did not know you can use it that way. Still not so familiar with JSON that much.

Thank you for being picky, that helps. :)