Code Critique Please

[Rv5]

Let me start out by saying this is actually c++ code, but I couldn't get
anyone on the c++ newsgroup to respond, and Id really like opinions. The
code works fine, so Im not looking for syntax help. Im more interested in
general programming practice and style critiques. I think it is good code,
but Ive said that and been wrong before. Despite it not being pure C, Im
hoping someone can still help me. One thing to keep in mind if you do look
is that there is no input error trapping. The teacher said its not
necessary so please keep that in mind. I normally would, in practice of
good coding techniques, but it wasn't necessary this time.

http://www.69chargerrt.com/comp322.htm

Thanks
Rv5

 

[Richard Heathfield]

Let me start out by saying this is actually c++ code, but I couldn't get
anyone on the c++ newsgroup to respond, and Id really like opinions. The
code works fine, so Im not looking for syntax help. Im more interested in
general programming practice and style critiques. I think it is good
code, but Ive said that and been wrong before.

Despite it not being pure C, Im
hoping someone can still help me.

I'd like to, but your code is really soaked in C++, and has no real
connection with the topic of comp.lang.c at all. I think you would do
better to await a response on comp.lang.c++.

 

[Jens.Toerring]

Let me start out by saying this is actually c++ code, but I couldn't get
anyone on the c++ newsgroup to respond, and Id really like opinions. The
code works fine, so Im not looking for syntax help. Im more interested in
general programming practice and style critiques. I think it is good code,
but Ive said that and been wrong before. Despite it not being pure C, Im
hoping someone can still help me. One thing to keep in mind if you do look
is that there is no input error trapping. The teacher said its not
necessary so please keep that in mind. I normally would, in practice of
good coding techniques, but it wasn't necessary this time.

http://www.69chargerrt.com/comp322.htm

I don't know much about C++ but since you're not using much more
than cin, cout and new I'll give it a try:

I don't like your using lots of global variables, and then with
names that don't tell what they are meant for (f, l and r). While
this is a short program where this may not hurt too much, it's bad
practice. If you don't want functions with lots of arguments simple
stick them into a single structure or class, so you can pass them
from main() easily. What makes things even worse is that you use the
same variable names also for local variables (see your use of the
variable 'l'), using them both! That makes things extremely confusing.

And you should check at least that the user wasn't lying when he/she
told you about the length of reference string - if the input value
is larger than the string length you'll be accessing elements of
the string aren't there (no idea what the string class will do in
these cases). But asking the user for the length is stupid anyway
since calculating the strings length is trivial.

Finally, many people see it as good coding practice to deallocate
all memory you allocated, so a call of delete for 'frame' and
'counter' wouldn't hurt.
Regards, Jens
--
_ _____ _____
| ||_ _||_ _| (e-mail address removed)-berlin.de
_ | | | | | |
| |_| | | | | | http://www.physik.fu-berlin.de/~toerring
\___/ens|_|homs|_|oerring