PHP Classes

Bad way to do things

Recommend this page to a friend!

      Lou Login  >  All threads  >  Bad way to do things  >  (Un) Subscribe thread alerts  
Subject:Bad way to do things
Summary:critics
Messages:4
Author:meivin123
Date:2014-09-21 20:51:56
 

  1. Bad way to do things   Reply   Report abuse  
Picture of meivin123 meivin123 - 2014-09-21 20:51:56
I've read parts of your code and it seems quite good.
But there are a few potential problems.
First youre using PDO prepared Statements.
But you directly concat the value in the sql string.
Instead you should use placeholders like descriped in the php manual.
Secondly md5 is outdated and not safe its better to use the new pwassword_hash() function or bcrypt in earlier versions.

  2. Re: Bad way to do things   Reply   Report abuse  
Picture of António Lourenço António Lourenço - 2014-09-21 21:49:24 - In reply to message 1 from meivin123
Hi meivin123!

I appreciate and accept constructive critical.
I'll try to improve the code.

STEP BY STEP, TOGETHER EVOLVED ...

António Lourenço

  3. Re: Bad way to do things   Reply   Report abuse  
Picture of meivin123 meivin123 - 2014-09-22 09:39:21 - In reply to message 2 from António Lourenço
Nice.
It would be better, too, if you dont instantiate the PDO object in inside your class but outside and then pass it to the constructor (called dependency injection).
So if you have bigger projects you instantiate it once and then pass it to all objects that need it.
So you dont have to change your database login inside your class and you maybe have the oportunity to use diffrent database drivers than mysql (If your used SQL syntax is supported on them).

  4. Re: Bad way to do things   Reply   Report abuse  
Picture of António Lourenço António Lourenço - 2014-09-22 16:39:17 - In reply to message 3 from meivin123
hi!
Thanks for the advice / tip.