##Too many if conditions A methods containing multiple if's smells. It makes testing harder since there are more edge cases. It also creates a mess where none should be.

##Only take what you need Every time you write SELECT *, a cute small kitten dies. You only need the userName, hash and salt.

#OO review Your code is OO, you are using Objects and stuff and the class keyword. But I think what you are asking is: is it good OO?

##No it isn't A good place to start is SOLID. Starting with Single responsibility.

Too many if conditions

A methods containing multiple if's smells. It makes testing harder since there are more edge cases. It also creates a mess where none should be.

Only take what you need

Every time you write SELECT *, a cute small kitten dies. You only need the userName, hash and salt.

OO review

Your code is OO, you are using Objects and stuff and the class keyword. But I think what you are asking is: is it good OO?

No it isn't

A good place to start is SOLID. Starting with Single responsibility.

##To many if A methods containing multiple if's smells. It makes testing harder since there are more edge cases. It also creates a mess where none should be.

##Too many if conditions A methods containing multiple if's smells. It makes testing harder since there are more edge cases. It also creates a mess where none should be.

This makes it a private variable (as it should be) instead of it being added on runtime (what you are doing now).

Then, your userLogin method also contains a lot of if's. And mo, you are giving away to much info. Don't tell what was wrong (username or password). Only tell that the combination is wrong. this way you don't give away any information. So:

##Only take what you need Every time you write SELECT * a cute small kitten dies. You only need the userName, hash and salt.

Then you should add some parameter binding spcies and your all good to go!

Then, what happens after is weird. We know that if a user exists, only 1 should. So we can add a LIMIT 1 statement to the sql. This way, the database will stop searching once it has found 1 member instead of continueing to search the entire members table.

Your checkPass function has the same problem. You evaluate something to a boolean. Then you evaluate that boolean in the if-condition and then you return an evaluated boolean. Better is:

A good place to start when writing OO-code is in starting with your controllers first. Here you should write how you would like to react with you aplpication. for instance:

This makes it a private variable (as it should be) instead of it being added on run-time (what you are doing now).

Then, your userLogin method also contains a lot of if's. And no, you are giving away to much info. Don't tell what was wrong (username or password). Only tell that the combination is wrong. This way, you don't give away any information. So:

##Only take what you need Every time you write SELECT *, a cute small kitten dies. You only need the userName, hash and salt.

Then you should add some parameter binding spices and your all good to go!

Then, what happens after is weird. We know that if a user exists, only 1 should. So we can add a LIMIT 1 statement to the SQL. This way, the database will stop searching once it has found 1 member instead of continuing to search the entire members table.

Your checkPass function has the same problem: you evaluate something to a boolean. Then you evaluate that boolean in the if-condition and then you return an evaluated boolean. A better method is:

A good place to start when writing OO-code is in starting with your controllers first. Here you should write how you would like to react with you application. for instance: