My Class file! Need Critique!
Moderator: General Moderators
My Class file! Need Critique!
solved thanks. Guys.
Last edited by karolismf on Sun Jul 29, 2012 8:16 am, edited 1 time in total.
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: My Class file! Need Critique!
Lots of code there. For me the two big issues are:
1. Lack of separation between the code that generated the output and the code that contains the business/domain logic
2. Not naming you objects as specific nouns. You have a class called "Check" which is about the most generic term possible. The name does describe what the object is. Likewise you use the work "accounts" which is also general. If they are user accounts then call the table and the object "users". Your naming leads to the wrong separations.
1. Lack of separation between the code that generated the output and the code that contains the business/domain logic
2. Not naming you objects as specific nouns. You have a class called "Check" which is about the most generic term possible. The name does describe what the object is. Likewise you use the work "accounts" which is also general. If they are user accounts then call the table and the object "users". Your naming leads to the wrong separations.
(#10850)
Re: My Class file! Need Critique!
Hmm, for first and secound issue i understood about more than half of it. Sorry, i don't have such great english. so if i understand it right you mean that in forgotPassword i have all code about forgotPassword and there is somecode in Check? it's unnecessary using from two classes code breaking? Hmm, first of all lets fix some things in my code and post fixed. Then we will move forward. here some fixes:
....
okey now look throught it again and tell me if i solved even one of your issue?
And thanks for help.
....
okey now look throught it again and tell me if i solved even one of your issue?
Last edited by karolismf on Sun Jul 29, 2012 8:17 am, edited 1 time in total.
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: My Class file! Need Critique!
This kind of discussion would be easier in your native language -- sorry about that.
My main comment is that classes are usually named using nouns and methods named using verbs. That makes class names describe what the object is and the data it contains. Using verbs makes methods describe what they do. It is even better if method names follow the "Tell Don't Ask" style if possible. Good naming comes from good decomposition of the the problem and the domain.
Microsoft
Class Naming: http://msdn.microsoft.com/en-us/library/4xhs4564(v=vs.71).aspx
Method Naming: http://msdn.microsoft.com/en-us/library/4df752aw(v=vs.71).aspx
Wikipedia
http://en.wikipedia.org/wiki/Naming_con ... ramming%29
My main comment is that classes are usually named using nouns and methods named using verbs. That makes class names describe what the object is and the data it contains. Using verbs makes methods describe what they do. It is even better if method names follow the "Tell Don't Ask" style if possible. Good naming comes from good decomposition of the the problem and the domain.
Microsoft
Class Naming: http://msdn.microsoft.com/en-us/library/4xhs4564(v=vs.71).aspx
Method Naming: http://msdn.microsoft.com/en-us/library/4df752aw(v=vs.71).aspx
Wikipedia
http://en.wikipedia.org/wiki/Naming_con ... ramming%29
(#10850)
Re: My Class file! Need Critique!
"My main comment is that classes are usually named using nouns and methods named using verbs" Is it something to do with the coding or the code will have holes if i don't fix this problem, or my web security is on risk if i don't change them?
if no, then please give me your own example how should it look like. I would compere to mine and i think it would be easier for both of us understand each other. And if yes, then i want an example for the best coded example in this situation.
Thanks. "This kind of discussion would be easier in your native language -- sorry about that. " Np. I am not very good in english far more worse than you, sorry for bad english.
(readed again, so you suggest to write the class names something diffrent from it's functions? like class forgotPassword/ so we name it arWt4dfWF)?
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: My Class file! Need Critique!
Security and good design are slightly different things. They do overlap, but you can have poorly designed code that is secure. It should be easier to understand and maintain well design code. That should lead to more secure code, but security is not guaranteed by good design. They are different.karolismf wrote:"My main comment is that classes are usually named using nouns and methods named using verbs" Is it something to do with the coding or the code will have holes if i don't fix this problem, or my web security is on risk if i don't change them?
Ok, let us start with forgotPassword. It does not seem like a single class. It seems more like a page on a website like "mysite.com/forgotPassword.php". Inside that page you need:karolismf wrote:if no, then please give me your own example how should it look like. I would compere to mine and i think it would be easier for both of us understand each other. And if yes, then i want an example for the best coded example in this situation.Thanks. "This kind of discussion would be easier in your native language -- sorry about that. " Np. I am not very good in english far more worse than you, sorry for bad english.
(readed again, so you suggest to write the class names something diffrent from it's functions? like class forgotPassword/ so we name it arWt4dfWF)?
1. code to determine what the page will do in its different possible states
2. code to access the database
3. code to display the page for the user and/or send an email to the user
How could you divide your current class into three classes that do those things?
(#10850)
Re: My Class file! Need Critique!
Hmm, well you are half right about forgotpassword.php, my site uses only index and it calls includes if there is string in url like mysite.com/?a=forgot, then it calls a file./// I created html pages for solution if success sending forgotPassword or failed. And ofcourse your new password was send html page. In my website there is only one class that uses the forgotPassword system it's in here. And ofcourse we must count the mailer class, which gets an information of forgotPassword that is filed by user and sends an email. In other words my forgotPassword at website uses two classes one is Class forgotPassword, secound Class Mailer. (My forgot password sends the $_POST['email'] to forgotPassword class - the class first of all filters the text then checks if the users filed information is valid and it isn't empty if that data is valid it checks database if the user filed information matches in database if yes, then in forgotPassword we call secound class name Mailer and call it's public function sendForgot using the information stored from user. Then the function do it's job and mail was send to your email. Then in email i got an mail from my website with a link for forgotPass reset, i use it and it opens my website which uses the include controler with link mysite.com/?something=...&... when pressed link it opens new file with stored information of "mysite.com/?something=...&..." it checks if this thing was really done.(he checks throught database) then he calls in forgotPassword again Class Mailer and this time it uses diffrent Mailer function with information stored from the link. The secound mail gives you new mail with new password.) Like again i didn't understand something in your post it was the last line.
If you have skype it would be great to write throught it becouse this is long conversation...
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: My Class file! Need Critique!
Yes, I understand what that class does, but all of those things should not be in one class. I asked you to divide your forgotPassword into (at least) three classes that each focus on only one thing.karolismf wrote:Like again i didn't understand something in your post it was the last line.
(#10850)
Re: My Class file! Need Critique!
k, now i get it. But why? is it more secure or better for the website and it's function quality?
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: My Class file! Need Critique!
You would need to read a lot about software design to truly understand why.karolismf wrote:k, now i get it. But why?
As I said above, you can produce a secure, functional application with poorly designed code. However, using proven design concepts like SOLID and best practices will make the code clearer, easier to change and easier to maintain. There are many, many ways that good design implicitly produces code that is easier to secure and tune. One of the hardest things for programmers to accept is that good design is often counter-intuitive. It is therefore at odds with the designs they have intuited.karolismf wrote:is it more secure or better for the website and it's function quality?
If decomposed the class as I suggested, you would start to see why...
(#10850)
Re: My Class file! Need Critique!
A couple of things:
1) Use curly brackets even when they're not necessary; it's more future proof.
2. Switch to the MySQLi extension or PDO to make use of prepared statements. This can speed up your application and will prevent SQL injections.
1) Use curly brackets even when they're not necessary; it's more future proof.
Code: Select all
// Bad
public function process()
{
if( $this->valid_token() && $this->valid_data() )
$this->register();
return count( $this->errors )? 0 : 1;
}
// Good
public function process()
{
if( $this->valid_token() && $this->valid_data() )
{
$this->register();
}
return count( $this->errors )? 0 : 1;
}Re: My Class file! Need Critique!
Hmm, i will try. to change everything as you said.
Anything else. And Christopher i will try to make more classes as you suggested. 
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: My Class file! Need Critique!
Not necessarily more. Make the classes nouns and the methods verbs. Follow DRY, SOLID, and Tell Don't Ask (google themkarolismf wrote:Anything else. And Christopher i will try to make more classes as you suggested.
(#10850)