Bad practice!

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

User avatar
cybaf
Forum Commoner
Posts: 89
Joined: Tue Oct 01, 2002 5:28 am
Location: Gothenburg Sweden

Post by cybaf »

I agree with McGruff that refactoring and the use of many fns is a good way to do OOP. However, I've made some very complex code with fns as the "selectAction()" above, and I've come to the state where I no longer understand what a function does. I'm not sure if this is just me or if it becomes bad when a function is nested with function calls which in turn are nested aswell. Sometimes a fn would make a call, and then the called fn will make another call and so forth. Then it becomes very difficult to remember all the things done on the way which will affect the end result in the first calling function.

ok, I hope this was not too confusing...:)

[edit]

another thing about the curly brackets discussion...

imo the brackets should be on the same line as the function. like:

Code: Select all

<?php

function someFunction() {
   print "blabla";
}

?>
my strongest argument for this is that when brackets are put on their own line, there will be a line ending without a semicolon. and as we all know, that could be a problem which is hard to find...:)

So as I see it. if a line does not end with a ";" or a "{" or "}" something is wrong.

[/edit]
//cybaf
lastcraft
Forum Commoner
Posts: 80
Joined: Sat Jul 12, 2003 10:31 pm
Location: London

Post by lastcraft »

Hi.

Details, details...

I don't think writing good code is all that mysterious. You know it when you have to read other people's. Writing good code has very little to do with indenting styles and case issues. Most reasonable formats are fine and if it is a real mess then use a pretty printer on it. Clear code is...
1) Objects that have a single job to do or are purely aggregators of other objects. Play the part of objects in discussions - "I am the authenticator and I want to talk to...". Good code starts with design effort.
2) Names the accurately describe the function/method even if a bit long. Buy a pocket thesaurus and perch it on top of the monitor.
3) Code has had at least two people involved in it, giving an outsider perspective. Code review, pairing and rotating people around help here. Called common ownership.
4) Code that is as clean as possible. Delete anything vague - ruthlessly.
5) It should work. Are your automated tests always at 100%?
If the will to write good code is there, the code will get better.

If that doesn't work buy Martin Fowler's Refactoring book.

yours, Marcus
eletrium
Forum Commoner
Posts: 34
Joined: Tue Feb 10, 2004 3:38 pm

Post by eletrium »

I can't help it...

"Then it becomes very difficult to remember all the things done on the way which will affect the end result in the first calling function. "

Uh, who cares what it does 4 layers down? The whole point of breaking things up into smaller functions is SPECIFICALLY to avoid the need to keep 5,000 lines of code in your head at once. Small functions inherently allow the developer to focus their thoughts on that small piece of code.

NOW, each function it calls needs to be intuitively obvious. I prefer writing ZERO comments in my code. Its much easier to write self commenting code. And in practice and based on feedback from several other developers, it is a more universal language.

Example (albeit longwinded, it is an extreme example)
(kind of non code specific)

Code: Select all

Funciton PerformSummationOnTextualData( aTextFileWithData ): Double
&#123;
  Duble Summation = 0;
  DataString String = nil;

  OpenTextFileWIthData( aTextFileWithData );
  CheckIntegrityofData();
  
  ReadDataStringFromFile( aTextFileWithData, DataString );
  while( DataString <> nil ) 
  &#123;
    AddDataToSummation( Summation );
    ReadDataStringFromFile( aTextFileWithData, DataString );
  &#125;

  return Summation;
&#125;
If you get a file open error, which part of this that is not working should be obvious. If the numbers are not adding up right, then you might have issues with your summation function. If you are not reading in all data, then the read string function might be the place to look.

The point is this, if you want the specifics on how each segment works, comments or no, a good programmer will want to look at the function anyways. So just make the code itself readable and properly named.

How should it be named? Well, go through code of your OWN that is 3+ months old and you will learn. It is simply a matter of time and experience and bumping into your own code months down the road.

As far as efficiencies of calling gunctions within functions and compiler directives such as loncall or shortcall and blah blah blah... forget about it. Compilers already break your code up and re-do it in the most efficient manner anyways (well, mostly). Your code is just a basis for a good compiler to work off of. It is nowhere near the actual final code in the exe.
User avatar
hawleyjr
BeerMod
Posts: 2170
Joined: Tue Jan 13, 2004 4:58 pm
Location: Jax FL & Spokane WA USA

echoing strings

Post by hawleyjr »

It drives me nuts when people use double quotes to declare strings with no variables in them. For instance:

Code: Select all

echo "Hello World";
Should be:

Code: Select all

echo 'Hello World';
There are a couple reasons why this bothers me. One is it is quicker for the server to go through the string with using the single quote. The reason this bothers me the most is you can't inbed variable between single quotes. When I see a line of code such as:

Code: Select all

$myString = 'Hello World';
I know it is just Hello World so I don't have to look for a $var inside of it.

When I do have strings that require variables I still don't use double quotes.

For instance:

Code: Select all

$user_name = 'James';

echo 'The user's name is '.$user_name;
Applications such as Ultaedit will display the variable in a different color then the string. It makes it easier to read then:

Code: Select all

$user_name = 'James';
echo "The user's name is $user_name";
Just a pet peeve. Part of my job is code quality control. Small things like this make a huge difference.
d3ad1ysp0rk
Forum Donator
Posts: 1661
Joined: Mon Oct 20, 2003 8:31 pm
Location: Maine, USA

Re: echoing strings

Post by d3ad1ysp0rk »

hawleyjr wrote:There are a couple reasons why this bothers me. One is it is quicker for the server to go through the string with using the single quote.
Says who?

It's like saying "a" is harder to read than "b". They're both 1 character.
User avatar
markl999
DevNet Resident
Posts: 1972
Joined: Thu Oct 16, 2003 5:49 pm
Location: Manchester (UK)

Post by markl999 »

But in echo "a"; the PHP engine has to check if there's any variables in there, with echo 'a'; it doesn't, so it's quicker ... not noticably quicker .. but still ... :o

On a related note, echo 'Hello ',$foo; is quicker than echo 'Hello '.$foo; ;)
User avatar
phice
Moderator
Posts: 1416
Joined: Sat Apr 20, 2002 3:14 pm
Location: Dallas, TX
Contact:

Post by phice »

I like to have atleast one tab on the left of the code for every line, so it looks pretty decent. Also, inside if/while/for/foreach/etc, I use 3 spacers instead of tabs.

Keeping things constant and inline is the best practice of PHP.
Image Image
User avatar
hawleyjr
BeerMod
Posts: 2170
Joined: Tue Jan 13, 2004 4:58 pm
Location: Jax FL & Spokane WA USA

Post by hawleyjr »

It may be an unnoticeable difference between reading 'a' and "a" for the server but reading other people's code it is much easier to read 'a' then "a".
d3ad1ysp0rk
Forum Donator
Posts: 1661
Joined: Mon Oct 20, 2003 8:31 pm
Location: Maine, USA

Post by d3ad1ysp0rk »

hawleyjr wrote:but reading other people's code it is much easier to read 'a' then "a".
That's opinion, I look at

Code: Select all

echo 'hello';
vs

Code: Select all

echo "hello";
and the double quotes are much easier IMO to see (especially when cluttered in other code)
User avatar
patrikG
DevNet Master
Posts: 4235
Joined: Thu Aug 15, 2002 5:53 am
Location: Sussex, UK

Post by patrikG »

I think we can settle on that quotes can be a matter of taste.

My personal taste is to have single-quotes.
1) because it forces you to have variables seperate (echo 'hello '.$var.') which keeps the code cleaner.
2) because PHP doesn't need to go through every echo or print statement in search for variables (i.e. performance).

Quote discussion settled? ;)
User avatar
hawleyjr
BeerMod
Posts: 2170
Joined: Tue Jan 13, 2004 4:58 pm
Location: Jax FL & Spokane WA USA

Post by hawleyjr »

One more quick point..... (sorry Patrik)

Code: Select all

echo "<table width="100%" border="1" cellspacing="0" cellpadding="0"><tr>
<td bgcolor="#996633">&nbsp;</td>
<td bgcolor="#996633">&nbsp;</td>
<td>&nbsp;</td>
<td>&nbsp;</td>
<td>&nbsp;</td></tr>
<tr><td bgcolor="#996633">&nbsp;</td>
<td bgcolor="#996633">&nbsp;</td>
<td>&nbsp;</td>
<td>&nbsp;</td>
<td>$$money</td></tr>
</table>";

or

Code: Select all

echo '<table width="100%" border="1" cellspacing="0" cellpadding="0">
<tr><td bgcolor="#996633">&nbsp;</td>
<td bgcolor="#996633">&nbsp;</td>
<td>&nbsp;</td>
<td>&nbsp;</td>
<td>&nbsp;</td></tr>
<tr><td bgcolor="#996633">&nbsp;</td>
<td bgcolor="#996633">&nbsp;</td>
<td>&nbsp;</td>
<td>&nbsp;</td>
<td>$'.$money.'</td></tr>
</table>';
Which would you rather read?
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post by timvw »

echo "<img src='foo.jpg' alt='nice foo' />";

is valid html too :)
d3ad1ysp0rk
Forum Donator
Posts: 1661
Joined: Mon Oct 20, 2003 8:31 pm
Location: Maine, USA

Post by d3ad1ysp0rk »

I'd rather read

Code: Select all

echo <<<EOT
<table width="100%" border="1" cellspacing="0" cellpadding="0"> 
<tr><td bgcolor="#996633"> </td> 
<td bgcolor="#996633"> </td> 
<td> </td> 
<td> </td> 
<td> </td></tr> 
<tr><td bgcolor="#996633"> </td> 
<td bgcolor="#996633"> </td> 
<td> </td> 
<td> </td> 
<td>$$money</td></tr> 
</table>
EOT;
:)
McGruff
DevNet Master
Posts: 2893
Joined: Thu Jan 30, 2003 8:26 pm
Location: Glasgow, Scotland

Post by McGruff »

If you read back a bit you'll find some posts about not mixing up html with php (ie keeping models and views separate). Now that is bad practice.

Worrying over double and single quotes is a bit of a waste of time.
eletrium
Forum Commoner
Posts: 34
Joined: Tue Feb 10, 2004 3:38 pm

Post by eletrium »

"Worrying over double and single quotes is a bit of a waste of time."


Ayup
Post Reply