Page 2 of 5

Posted: Sat Mar 06, 2004 10:27 pm
by lastcraft
Hi...

Code doesn't really annoy me any more by itself. We are all beginners after all. Attitude annoys me though :x . One annoyed me so much it is laughable (after the event).

I had to work on some offshore code. Before the code was written we had shipped them a couple of libraries, a simple persistence library and a widget set. We asked them to use these rather than direct SQL access and to not use templates as the graphic designer and stakeholder was quite happy working the HTML with PHP widgets and CSS through a text editor. We would be separating the model from presentation after all, which is the main idea.

After shipping the libraries my role was ended, or so it was hoped. Some eight weeks later (a full four late) the site of sorts was delivered. It was just the same structure as previous sample code they had sent. It had templates and a wry comment to the effect that the previous developer (me) was a beginner for not doing it like this. Rather than use the widget classes to keep the state of the controls between requests, fields had been hand coded with random results. They also created a scripty class loader that required four files to load the one wanted. At the end of which a class for each page would be loaded that had a single method of 300-400 lines, with bits of printed HTML and hard coded SQL. No other classes! Add in the template system and a 10 page web project had become over 100 files.

To add just one extra page to the site (one that was missing) involved editing five files with zero reuse. It took me a whole day to figure out this mess and three more to fix the bugs. When we asked them to explain this curious set-up I was told that the developer gave lectures at a university and it was obviously too clever for us. Over the next hour several unsolicited insulting e-mails followed to the stakeholder (who I was sitting next to) that I was unprofessional to be editing code I didn't understand. Classic.

Determined ignorance...hmm...that annoys me.

yours, Marcus

Posted: Sun Mar 07, 2004 12:40 am
by eletrium
"What would you consider to be bad practice when coding PHP? "

1) Writing crappy code is bad practice. What are widely accepted examples of crappy code? Look at Chapter 3(?) of Martin Fowler's "Refactoring" book. The one about bad smells in code.

2) Arguing over six lines of simple code:

<?php
function printhdr($Param)
{
echo "Whatever " . $Param . " rest of line<br><br>";
}
?>

Your job is to complete a task that makes a customer money. Actual cost is directly dependent upon hours worked on it. Ask yourself: If I was paying this guy's salary, would I want him debating over these lines of code or just go with it and get the next part of the project done? If you answer yes, do a reality check and calculate the cost to you for the guy argueing that.

Posted: Sun Mar 07, 2004 12:43 am
by eletrium
"Code doesn't really annoy me any more by itself. "

Code does not annoy me either. But the moronic programmers who wrote it sure bug the <span style='color:blue' title='I&#39;m naughty, are you naughty?'>smurf</span> out of me.

And for the record in most cases "moronic programmers" refer to myself. Paging Dr. Freud. (It's embarassing looking at a piece of code you wrote 18 months ago and asking yourself "What the hell was this moron thinking?" then seeing your initials in a comment after it)

Posted: Sun Mar 07, 2004 3:58 am
by m3mn0n
Here is an example of what I mean't by unnessesary use of functions:

Code: Select all

<?php
function close_it ()
{

    mysql_close();

}
?>
Or

Code: Select all

<?php
function echo_this ($text)
{

    echo ($text);

}
?>
heh, the second is a bit over the top, but the first one I see often.

Posted: Sun Mar 07, 2004 5:16 am
by McGruff
There could be a good reason for close_it(): wrapping native db fns allows you to easily switch databases. A handful of quick fn edits compared to sifting through hundreds of db ops in dozens of files (assuming the all the sql is tranferrable).

There could even be a similar reason for echo_text(). Perhaps it's a template fn which will be used to apply some formatting to page vars - in its current state it does nothing but again one fn edit would instantly update every script which uses the fn.

Granted, that might not be the case at all but I'm deliberately making a noise about this because I think extensive use of fns and classes is an important step up from the type of spaghetti coding we all do when we're starting out. It's the difference between having your face stuck in the detail and sitting back and thinking about design.

Good design means looking for a clean separation of the different threads in a script (business logic & presentation being one of the main fault lines, as mentioned above) and the discipline of writing short, single-task functions and classes is crucial to disentangling a script.

Posted: Sun Mar 07, 2004 10:36 am
by patrikG
I agree with McGruff - dedicating one function to one rather than a plethora of tasks helps you to
a) break down the logic of your application into it's smallest or nuclear elements (note: nuclear as in nuclear family) ;)
b) easily identify where those nuclear logcial tasks overlap and thus can share a function

You end up with an application that is readable, easily understandable even by the layperson and, in the best of cases, even portable to other languages.

There is, of course, the danger of "overnuking", i.e. having a totally fragmented set of functions/classes - which started off a as nice, coherent set of applied logic fizzled somewhere and became a mass of half-baked code.
That is where
a) design patterns can help
b) diagrams (as much as I hate doing them) come in handy.
eletrium wrote:Your job is to complete a task that makes a customer money. Actual cost is directly dependent upon hours worked on it. Ask yourself: If I was paying this guy's salary, would I want him debating over these lines of code or just go with it and get the next part of the project done? If you answer yes, do a reality check and calculate the cost to you for the guy argueing that.
The point is rather: how much of your time do you want to spend on solving deja vu problems that you've already solved so many times before.
I want to spend the minimum of my time on that - and that is why creating re-usable code is so important. When you start thinking not just about, e.g. your own database wrapper and HTML drop-down box building function, you start wanting to build applications rather than code monstrosities which you would dread to revisit.
That's the point of avoiding quick fixes and thinking about re-usable code.

And yes, I agree with you, eletrium, that my statements above do need a reality check. Time is something that always comes in on any project. When you have the pleasure of doing a project from start to finish, you can implement what I said above.
When you're called in to finish a project someone else has half-finished it depends very much on the circumstances if you would have the time and liberty to do that.

*Edit* P.S.: Thanks, McGruff, for putting the thread into the more appropriate forum.

Posted: Sun Mar 07, 2004 6:32 pm
by eletrium
"b) diagrams (as much as I hate doing them) come in handy. "

Totally agree. I think especially if you use them as a brief tool to gather your thoughts. Meaning don't worry how neat it is or anything like that or even if it is the appropriate UML stuff. Just sort of back of the envelope kind of stuff.


============
"That's the point of avoiding quick fixes and thinking about re-usable code."

My comment in no way advocated a quick fix. It advocated asking yourself if it was cost efficient. If five months later you are going to spend a week of time trying to figure out what you were thinking its cost efficient to re-write it in a more readable fashion.

=============
Your "comments above" PatrikG

"I agree Sami & McGruff, also what I find "highly improvable", i.e. annoying, is when you read code and it's all in those one-character variables $a to $z. At best maybe even two characters.

Jeez, the machine understands the code anyway, but us humans have to maintain it. Make the code as readable as possible and if you return to a piece of code written 10 months ago, you'll find your way around much more easily."

I totally agree with these comments. "Does it really matter to the customer if the variable is one letter?" Answer: Yup. They will never know what they really want the program to do up front, so you will always have to go back and re-do parts. Making your code readable imho falls into the cost efficient category. I have been down both paths several times each, and it's just best to rename variables that make no sense in any way.

But debating the use of a one line function seems like a waste of time. There are very good arguments on both sides of the aisle, and neither side is going to budge. I have my own opinions about that topic, and I know very good programmers who have opposing opinions.

I don't know a single programmer who advocates writing unreadable code however. PatrikG I totally agree with your post.

Posted: Sun Mar 07, 2004 11:20 pm
by Bill H
Sami, it looks like you and I are a minority of two.

Posted: Mon Mar 08, 2004 4:21 am
by twigletmac
I like functions, now if I could get my fellow developers to use my function library instead of reinventing the wheel continuously I'd be happy.

However, poorly named functions and non-documented functions really get on my wick - what do you think a function called check_year() does (unfortunately it's not the obvious)?

I don't understand developers who try and squeeze all their code into as few lines as possible even going so far as to have 3 or 4 bits of code all separated by semi-colons on one line...

Mac

Posted: Mon Mar 08, 2004 4:34 am
by malcolmboston
lol well something that really gets my back up is

Code: Select all

if ($var >= 1) {
print "do something"; }
personally my scripts look very clean and are instanly obvious what they are doing, i do it all like this:

Code: Select all

if ($var >= 1) 
{
print "
do something
";
}
pretty pointless, but what im saying is unclear code

also people that write 100's of lines of PHP code without any commenting
if i write a 500 line script around 50 of the lines will be comments

Posted: Mon Mar 08, 2004 1:50 pm
by d3ad1ysp0rk
eeck..

Isn't
print "
do something
";
bad syntax anyways?

Code: Select all

if($var >= 1){
echo "Do something.";
}

Posted: Mon Mar 08, 2004 3:05 pm
by Ixplodestuff8
I like giving the braces their own line, (but not the print statement) like so:

Code: Select all

<?php
if ( $var >= 1)
{
    print 'Do something';
}
?>
also indent everything inside the brackets with a tab (or four spaces if I can't tab for whatever reason, like the forum text box).

and I after I type in the opening brace I always immediatly press enter twice, and close it, before putting anything inside, that way I never miss a closing tag.

Posted: Tue Mar 09, 2004 2:58 pm
by Steveo31
I think unclear code is a biggie, and I don't really like using the C type of coding in which an opening bracket get's its own line:

Code: Select all

function foo()
{
    doSomething();
}
Looks bad IMO and sometimes I can't tell if it's one opening or closing. I realize that there's only one right way of doin it (as in there has to be a closing bracket for each open one), and with large resolution screens, those brackets can be pretty small. Besides, if I do it on the same line:

Code: Select all

function foo(){
    doSomething();
}
I can tell quicker and easier if the first bracket is open "foo(){" rather than "foo()}". Who knows.. the finger sometimes doesn't go the extra 1/4" for the other bracket.

Commenting is a big one, but if the code is simple (as in short) then I don't see it useful. I agree with elitrium... var names are pretty crucial.

[edit]

Oh yea... mySQL stuff- the commands being lowercase, as in

Code: Select all

select something from somewhere
As opposed to:

Code: Select all

SELECT something FROM somewhere
Depending on the names of the values you are after, it could be highly confusing if they were lowercase.

[/edit]
.02

Posted: Wed Mar 10, 2004 2:29 am
by JAM
I thought this area of discussion was huge... Lately I realized that it's really huge...

If I'd discuss my own code, I likely write semi-bad code (if others would view it) on my private projects as oly I need to understand it and 'good code' on public projects.
I'm using both replying to posts on this and other boards as I tend to keep it at the issue-level (likely wrong code) rather than trying designing it. Obvious bad practice written code would be corrected of course.

Afaik, there will never be a common standard, no matter how long we tell people that indents are 4 spaces not a [tab], brackets should be used as opposed to not always needed, placement of the same, heredoc vs. echo, welldocumented code, understandable function names aso. aso.

I personally hate heredoc but know how to use it and will use it if others in my projects do, but should I consider that bad practice if I think it's overused? Who am I to tell when it's overused?
...so how do anyone tell where the line begin between bad and good code start and end?

Posted: Wed Mar 10, 2004 4:23 am
by llanitedave
I've seen the brackets debate in a lot of forums -- I'm sure it's mostly a matter of personal preference.

I've tried the

Code: Select all

/**** comment about the function ****/
function change_data ($data) // short optional comment
{
    if (good) // another short optional comment
    {
        return $unchanged;
    }
    else // still another comment
    {
         return $changed;
    }
}
style, but I ended up confusing myself a lot. I kept looking for semicolons above the opening bracket.

I find that

Code: Select all

// Comment about function
function change_data ($data) { // short optional comment
    
    if (good) { // another short optional comment
        return $unchanged;

    } else { // still another comment
         return $changed;

    }

}
Is a little easier for me to read.

The "else" block, however, always bites me in the butt no matter what I do!