470,815 Members | 1,335 Online
Bytes | Developer Community
New Post

Home Posts Topics Members FAQ

Post your question to a community of 470,815 developers. It's quick & easy.

Re: Newbies Code: Please critique

Mo wrote:
$fiveCells = "<td></td<td></td<td></td<td></td<td></td>";
Don't. If you need spacers in a table, do:

echo "<td colspan='5'></td>";
$empContent = '<td class="empName">' .
$empRow["signature"] . "</td>";
$empContent .= "<td>OrderCount</td>";
$empContent .= "<td>GrossSales</td>";
$empContent .= "<td>GrossCosts</td>";
$empContent .= "<td>GrossProfits</td>";
print $empContent;
You will compact your code and make it more legible if you just output
everything from the beginning in those cases. There will be situations in
which you'll need to delay the output (as discussed), but in the code you
sent, there are lots of constructs like the one above. Try to avoid that if
you can.

Besides that, it's a long piece of spaghetti code - long, with no functions,
and quite a lot of nested loops. That, in the long run, tends to make code
ilegible. Please try to design your code so that it is more modular and
less spaghetti.

Cheers,
--
----------------------------------
Iván Sánchez Ortega -ivansanchez-algarroba-escomposlinux-punto-org-

Now listening to: Fly Gang - Calambre Techno - Te Gira la Cabeza (disc 1:
Sistema de Sonido) (1999) - [7] Feeling pt 1 (5:35) (86.750000%)
Jun 27 '08 #1
7 1288
Mo
On Jun 17, 4:24 pm, Ivn Snchez Ortega <ivansanchez-...@rroba-
escomposlinux.-.punto.-.orgwrote:
Mo wrote:
$fiveCells = "<td></td<td></td<td></td<td></td<td></td>";

Don't. If you need spacers in a table, do:

echo "<td colspan='5'></td>";
$empContent = '<td class="empName">' .
$empRow["signature"] . "</td>";
$empContent .= "<td>OrderCount</td>";
$empContent .= "<td>GrossSales</td>";
$empContent .= "<td>GrossCosts</td>";
$empContent .= "<td>GrossProfits</td>";
print $empContent;

You will compact your code and make it more legible if you just output
everything from the beginning in those cases. There will be situations in
which you'll need to delay the output (as discussed), but in the code you
sent, there are lots of constructs like the one above. Try to avoid that if
you can.

Besides that, it's a long piece of spaghetti code - long, with no functions,
and quite a lot of nested loops. That, in the long run, tends to make code
ilegible. Please try to design your code so that it is more modular and
less spaghetti.

Cheers,
--
----------------------------------
Ivn Snchez Ortega -ivansanchez-algarroba-escomposlinux-punto-org-

Now listening to: Fly Gang - Calambre Techno - Te Gira la Cabeza (disc 1:
Sistema de Sonido) (1999) - [7] Feeling pt 1 (5:35) (86.750000%)
The reason I avoided using colspan is because I've seen it cause
problems when it comes to trying to export the data to a .CSV format.
I do plan on getting this report to the point where that is an feature/
option, and am just trying to plan ahead.
Any further advice?

As for making the spaghetti code more modular, do you have any
suggestions?
What parts would be prime candidates for being written into a
function?
Also, are you referring breaking up the code into multiple pages and
using INCLUDE function? (I do plan to do this with the DB connection
parts of the code.)

Thanks.
~Mo

~Mo
Jun 27 '08 #2
Mo wrote:

[...]
The reason I avoided using colspan is because I've seen it cause
problems when it comes to trying to export the data to a .CSV format.
Uh, what?

How (on earth) are you doing the html -csv conversion?

It's much easier, IMHO, to use a MVC pattern to output either HTML or CSV.
As for making the spaghetti code more modular, do you have any
suggestions?
Not really: how the code is structured depends on the design, not on the
coding itself. I do think that reading about design patterns would
enlighten you.
Also, are you referring breaking up the code into multiple pages and
using INCLUDE function? (I do plan to do this with the DB connection
parts of the code.)
Yes, but you really need to plan that in advance, and for planning it
*right*, you'll just need some experience.

Cheers,
--
----------------------------------
Iván Sánchez Ortega -ivansanchez-algarroba-escomposlinux-punto-org-

Now listening to: Chambers Brothers - CSI: Crime Scene Investigation
(2002) - [11] Time Has Come Today (4:52) (87.444397%)
Jun 27 '08 #3
Iván Sánchez Ortega schreef:
Mo wrote:

[...]
>The reason I avoided using colspan is because I've seen it cause
problems when it comes to trying to export the data to a .CSV format.

Uh, what?

How (on earth) are you doing the html -csv conversion?

It's much easier, IMHO, to use a MVC pattern to output either HTML or CSV.
>As for making the spaghetti code more modular, do you have any
suggestions?

Not really: how the code is structured depends on the design, not on the
coding itself. I do think that reading about design patterns would
enlighten you.
>Also, are you referring breaking up the code into multiple pages and
using INCLUDE function? (I do plan to do this with the DB connection
parts of the code.)

Yes, but you really need to plan that in advance, and for planning it
*right*, you'll just need some experience.

Cheers,
If I may drop in. ;-)

Ivan, first: I totally agree with the suggestions you made to improve
his code. Sound advise.

But I really don't think it is good advise to throw MVC and
designpatterns at a (selfdeclared) newbie.
In my humble opinion every new coder needs to make a lot of own mistakes
before possibly seeing the merrit of such approaches.
It is like teaching somebody math Integral transformations first, and
adding and subtracting later.

[private opinion]
About design patterns I have a very personal opinion that they do not
add to the creativity and insight of the programmer using them. It is
more like a cookbook, and the programmer has to try to form/restate
his/her problem in such a way that a design pattern can be applied.
Every starting programmer has the right to make multiple mistakes and
learn from them: it activates selfcritism and some paranoia, which makes
you a better programmer. Reading about/using design patterns can be good
fun and a source of inspiration to an experienced programmer, but don't
throw them at starters.
Feel free to shoot me now. ;-)
[/private opinion]

Regards,
Erwin Moller
Jun 27 '08 #4
Mo wrote:
On Jun 17, 4:24 pm, Ivn Snchez Ortega <ivansanchez-...@rroba-
escomposlinux.-.punto.-.orgwrote:
>Mo wrote:
>>$fiveCells = "<td></td<td></td<td></td<td></td<td></td>";
Don't. If you need spacers in a table, do:

echo "<td colspan='5'></td>";
>> $empContent = '<td class="empName">' .
$empRow["signature"] . "</td>";
$empContent .= "<td>OrderCount</td>";
$empContent .= "<td>GrossSales</td>";
$empContent .= "<td>GrossCosts</td>";
$empContent .= "<td>GrossProfits</td>";
print $empContent;
You will compact your code and make it more legible if you just output
everything from the beginning in those cases. There will be situations in
which you'll need to delay the output (as discussed), but in the code you
sent, there are lots of constructs like the one above. Try to avoid that if
you can.

Besides that, it's a long piece of spaghetti code - long, with no functions,
and quite a lot of nested loops. That, in the long run, tends to make code
ilegible. Please try to design your code so that it is more modular and
less spaghetti.

Cheers,
--
----------------------------------
Ivn Snchez Ortega -ivansanchez-algarroba-escomposlinux-punto-org-

Now listening to: Fly Gang - Calambre Techno - Te Gira la Cabeza (disc 1:
Sistema de Sonido) (1999) - [7] Feeling pt 1 (5:35) (86.750000%)

The reason I avoided using colspan is because I've seen it cause
problems when it comes to trying to export the data to a .CSV format.
I do plan on getting this report to the point where that is an feature/
option, and am just trying to plan ahead.
Any further advice?

As for making the spaghetti code more modular, do you have any
suggestions?
What parts would be prime candidates for being written into a
function?
I'm just learning php, so some of my "advice" may be a bit off.

You have a mix of collating strings and just echoing them out a line at
a time. Consider using a heredoc to minimize.

echo <<<EOT
<tr>$empRow["signature"]<td><td>OrderCount</td><td>GrossSales</td>...
....
EOT;

It looks to me that most of your table could be in a single heredoc. It
will be much easier to read and organize your html.

You have a lot of loops where you create a new query, consider
preparing your sql outside the loops and doing the executing inside. You
may wish to look at PDO and prepare.
Break as much as you can into functions (or classes), you'll be heading
in that direction sooner or later anyways.

Jeff

Also, are you referring breaking up the code into multiple pages and
using INCLUDE function? (I do plan to do this with the DB connection
parts of the code.)

Thanks.
~Mo

~Mo
Jun 27 '08 #5
Erwin Moller wrote:

[...]
But I really don't think it is good advise to throw MVC and
designpatterns at a (selfdeclared) newbie.
In my humble opinion every new coder needs to make a lot of own mistakes
before possibly seeing the merrit of such approaches.
It is like teaching somebody math Integral transformations first, and
adding and subtracting later.
I agree with you, but I have to ask:

What should be appropiate for a coder with this little experience? OOP
principles? Basic algorithmics? Functional programming principles?
Flowcharting?
[...] It is more like a cookbook, and the programmer has to try to
form/restate his/her problem in such a way that a design pattern can be
applied.
IMHO, one should never implement a design pattern "as per the book". They're
not recipes, they're ideas - they help you to devise new ways to look at
your problem, and new approaches to build complex systems.

OK, maybe they are a bit overkill, but one has to start making errors
somewhere to get experience, right? ;-)
Feel free to shoot me now. ;-)
/me loads his shotgun

:-P

--
----------------------------------
Iván Sánchez Ortega -ivansanchez-algarroba-escomposlinux-punto-org-

Un ordenador no es un televisor ni un microondas, es una herramienta
compleja.
Jun 27 '08 #6
Iván Sánchez Ortega wrote:
Mo wrote:

[...]
>The reason I avoided using colspan is because I've seen it cause
problems when it comes to trying to export the data to a .CSV format.

Uh, what?

How (on earth) are you doing the html -csv conversion?

It's much easier, IMHO, to use a MVC pattern to output either HTML or CSV.
>As for making the spaghetti code more modular, do you have any
suggestions?

Not really: how the code is structured depends on the design, not on the
coding itself. I do think that reading about design patterns would
enlighten you.
I'm a new PHP programmer, but not new at programming. I'm unfamiliar
with this. Could you throw an example or a link my way? What I've just
googled up makes this look like a RAD. When I look at PHP Cake examples,
it looks like a collection of classes. I'm really confused.

My own style is to use a template and call a class depending on what
the page instructions are. The end result is to have reusable bits that
can be customized and reconfigured. Instructions could be sent on the
query string or be related to the page path. I'm not sure how that fits
in with this.

Jeff
>
>Also, are you referring breaking up the code into multiple pages and
using INCLUDE function? (I do plan to do this with the DB connection
parts of the code.)

Yes, but you really need to plan that in advance, and for planning it
*right*, you'll just need some experience.

Cheers,
Jun 27 '08 #7
Jeff wrote:
I'm a new PHP programmer, but not new at programming. I'm unfamiliar
with this. Could you throw an example or a link my way?
Here you go:
http://en.wikipedia.org/wiki/Design_...mputer_science)
http://en.wikipedia.org/wiki/Design_Patterns
http://en.wikipedia.org/wiki/Code_Complete
http://en.wikipedia.org/wiki/Anti-pattern

I think you'll have enough research material ;-)

--
----------------------------------
Iván Sánchez Ortega -ivansanchez-algarroba-escomposlinux-punto-org-

Un ordenador no es un televisor ni un microondas, es una herramienta
compleja.
Jun 27 '08 #8

This discussion thread is closed

Replies have been disabled for this discussion.

Similar topics

37 posts views Thread by Eric | last post: by
19 posts views Thread by TC | last post: by
8 posts views Thread by G Patel | last post: by
188 posts views Thread by christopher diggins | last post: by
reply views Thread by floortje | last post: by
2 posts views Thread by winston | last post: by
2 posts views Thread by matt | last post: by
reply views Thread by mihailmihai484 | last post: by
By using this site, you agree to our Privacy Policy and Terms of Use.