Menu
▾
▴
Re: [Dev-C++] info
|
From: Per W. <pw...@ia...> - 2008-04-04 19:25:42
|
You wrote: "As I have included some information and comments within the
code. With these information and short email messages should be able to
know what the intention was."
Please post normal ASCII mails to this mailing list.
Your original mail was multi-part. One part contained quoted-printable
without any line breaks (!), i.e. everything on a single line. Try to read
your header text (and the "table" to the right) if it's on a single
line...
The other part was quoted-printable HTML, i.e. looked something like:
/**************************************************************************=
******************************************************<BR> *
Des=
cription: Each star stands for a digit, with the leftmost star on
eac=
h &n=
bsp;  =
; &n=
bsp;  =
; &n=
bsp;  =
Note that every space in your text got converted to 6 characters in HTML
encoding. That is quite wasteful of bandwidth. Your mail many times larger
than the real information it contained.
Further: Your original source code must have contained tab characters (and
most probably very short tab characters too) that got converted to
non-break spaces. But not to the correct number of spaces since the mail
program did not know your true tab length.
Now, you expect people to spend any time trying to read your description?
Do post ASCII mails if you care about people spending time to read them.
And make sure that your mails does not contain any tab characters that may
get the indentation totally mangled!
You write: "This is not an assignment." If this is not a school
assignment, then what is it? This type of code does not produce any really
useful result. This is a coding project to let you exercice algorithms and
logic thinking. It is then reasonable to think that this is either school
work or an Internet coding contest, such as Project Euler. School work is
expected to be done on your own. Coding contests looses most of their
meaning unless the problems are solved individually...
About your for-loop functions:
I said that your for loops didn't work. A reasonable thing to do then
would be to take a closer look at them. If you did that but didn't see any
problem with them, the next thing would be to assume that I was wrong, or
to write a little test program and try them. Calling them 10 times in a
row and printing their return value would have made it obvious that the
loops did not perform any loop usable to your program.
About your code:
Before your loops, your comment says: "for() loops are used to randomly
assign a digit to each of the [...]". What is random about a for loop?
You have two three-digit numbers, i.e. the first digit in each number must
be non-zero. You have implemented 6 loops to iterate through all
combinations. But if only the first digit in each number must be non-zero,
why are all loops iterating from 9 down to 1? Shouldn't four of the six
loops include the digit zero?
And why do you have two variables for each loop? Why not iterate a1
directly, instead of iterating on i, and assign i to a1?
Note that you can convert the six loops into just two with:
for (a = 100; a <= 999; a++) {
a1 = a/100;
a2 = (a/10)%10;
a3 = a%10;
for (b = 100; b <= 999; b++) {
b1 = b/100;
b2 = (b/10)%10;
b3 = b%10;
// Do all work here.
}
}
And since you are only using a1, a2 and a3 to form mltply_1 (which the
above code has as loop variable) you can skip evaluating a1, a2 and a3
totally.
And if you swap places of the a and b loops, you don't need to evaluate
b1, b2, b3 in the inner loop.
You have a comment: "The result is assigned by multiplying both variable
mltply_1 and mltply_2". Don't you expect anyone looking at your code to
recognize that from the following source line?
result = mltply_1 * mltply_2;
Do not spend time writing comments that tells what individual source lines
does. With correct variable and function names, that should be obvious
from just looking at the code line. In some situations it may be needed to
document _why_ you are doing something, since the reason behind isn't
directly visible. Only when you have larger blocks of non-obvious code
will it be needed to write a 'summary' to describe what the code does - or
is expected to do.
Back to your code again. Why are you performing the test:
if ((mltply_1 * b3 < 1000) || ...
before evaluating partial_1, partial_2, partial_3?
If you assign your variables first, you don't have to evaluate the same
thing twice.
A selective - but very fast - test in your code is the requirement:
if (partial_3 == (partial_1 + partial_2)) ...
Why do you do that just before the printout? Wouldn't it be better to do
it before you spend a huge amount of work to split partial_1, partial_2,
partial_3 into 12 digits?
Your test of digits verifies that all digits in partial_1 are represented
in partial_2. But no test to verify that all digits in partial_2 are
represented in partial_1.
Why do you need 9 temporary variables just to split the partials into
digits? You do not make use of the intermediate values anywhere...
mltply_1, mltply_2, result, partial_1, partial_2 and partial_3 are
unsigned integers. You have used the formatting character %d which is for
signed integers.
My suggestions should allow you to remove between half and two thirds of
the variables, code lines and indentation levels and probably cut the
running time with a factor 10.
If the goal is to just get an answer, this doesn't matter. But then, this
isn't the type of problem that you solve just to get an asnwer. It is a
problem you solve to train your thinking, so the above improvements are
important. The goal isn't to find a solution, but to find a good solution.
Even more can be done to improve the code, but I must leave a bit for you
to think about too :)
An important rule: As soon as the code starts to look really messy, it is
time to stop what you are doing and start pondering if you are doing
something wrong. Most of the time, you are! Most programming problems are
intended to have a clever solution.
/pwm
|
