derangement: code review request


R

Richard Bos

Tim Rentsch said:
A couple of the names use abbreviations as part of the name (eg,
'fam_size'). I recommend using the whole word, and always avoiding
abbreviations.

I disagree. Even for normal abbreviations, not just for acronyms,
industry-standard abbrevs are probably more legible than the whole word.

I mean, my main program deals with entering data for advertisement. Can
you imagine how much larger and less legible my code would've been if
I'd used advertisement_width_in_columns everywhere I know have adv_cols?

Ew, ganoo style.
generate_random_derangement( unsigned a[], unsigned n ){
unsigned i, j, t, r, r_limit;

r_limit = RAND_MAX - RAND_MAX % n;

for( i = 0; i < n; i++ ) a = i;


Ew ew ew! You just proved that whitespace isn't _always_ good (and
whitespace on the _inside_ or parens rarely is). How about this more
moderate version:

for (i = 0; i < n; i++) a = i;

In fact, I'd prefer

for (i=0; i<n; i++) a=i;

But double spaces? Yuck.

Richard
 
Ad

Advertisements

M

Merrill & Michele

I think there exists enough information in this thread in order to take a
swipe at coding this.

I have 3 libraries to include, all of which are 1989-, 1999- and
2004-compliant. I have two macros. Mr. Pop's RND was not even close to
right. His SWAP I will lift from him wholesale. Persons like me who have,
at other times and places, used a swap command will see that this macro not
only does this but will squack (<<can't spell; rhymes with talk) if the
types aren't right. fam_size becomes FAMSIZ. I reject strongly the notion
that FAMILYSIZE would be a better option here. I guess I would add on to
the style comments above that one uses no macro name, which, parsed, yields
a keyword in most every language. Furthermore, seven letters is plenty for
my prog.variable.names . Let's give this a whirl, Earl:

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP danscode

int main(){

printf("tja\n");
return 0;
}
 
D

Dan Pop

In said:
You're amputating something that requires a band-aid. By throwing out rand
calls that exceed top_num above, the events mod FAMSIZ are equiprobable.
Period. I'll add white-spacing so as to make that statement clearer.

White space or not, I don't understand what you're trying to say. Try
something better than adding white space. Also make sure that what you're
actually trying to say is true.
2. If the above is not possible, write your own program to replace each
TAB character by as many spaces as needed to implement TAB stops every
N characters. It is an excellent exercise for you, at your current
level.

Following one of your earlier posts, I determined that I can direct the
input on the command line of this platform which will not be named, and thus
get text in and out of these progs in a sensible manner. The conversion
might be six lines of code and is behind me in K&R.
compute
a random shift value in the range 1 .. FAMSIZ-1 and relocate each member
to position (i + shift) % FAMSIZ:

shift = rand() % (FAMSIZ-1) + 1;
for (i = 0; i < FAMSIZ; i++) members = (i + shift) % FAMSIZ;

The derangement is still perfect.

So is members = members[i+1] for the the way we initialized.


True, but the results are predictible, while my method picks one of
the FAMSIZ-1 trivial derangements at random.


This statement contradicts a previous of yours.


It always helps if you point out the contradicted statement. AFAICT,
it doesn't contradict any of my previous statements.

Dan
 
M

Merrill & Michele

I think there exists enough information in this thread in order to take a
swipe at coding this.

I have 3 libraries to include, all of which are 1989-, 1999- and
2004-compliant. I have two macros. Mr. Pop's RND was not even close to
right. His SWAP I will lift from him wholesale. Persons like me who have,
at other times and places, used a swap command will see that this macro not
only does this but will squack (<<can't spell; rhymes with talk) if the
types aren't right. fam_size becomes FAMSIZ. I reject strongly the notion
that FAMILYSIZE would be a better option here. I guess I would add on to
the style comments above that one uses no macro name, which, parsed, yields
a keyword in most every language. Furthermore, seven letters is plenty for
my prog.variable.names . Let's give this a whirl, Earl:

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP danscode

int main(){

printf("tja\n");
return 0;
}

This compiles and links, not surprising to most. But what differs is that I
copypasted this off the net. Output:
http://home.comcast.net/~beckjensen/derange.htm

Next, I add the SWAP and do declarations. MPJ
 
M

Merrill & Michele

_leroy
I think there exists enough information in this thread in order to take a
swipe at coding this.

I have 3 libraries to include, all of which are 1989-, 1999- and
2004-compliant. I have two macros. Mr. Pop's RND was not even close to
right. His SWAP I will lift from him wholesale. Persons like me who have,
at other times and places, used a swap command will see that this macro not
only does this but will squack (<<can't spell; rhymes with talk) if the
types aren't right. fam_size becomes FAMSIZ. I reject strongly the notion
that FAMILYSIZE would be a better option here. I guess I would add on to
the style comments above that one uses no macro name, which, parsed, yields
a keyword in most every language. Furthermore, seven letters is plenty for
my prog.variable.names . Let's give this a whirl, Earl:

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP \
(tmp=buys_for[m],buys_for[m]=buys_for[n],buys_for[n]=tmp)
#define > ' '
int main(){

int i;
int m,n,tmp;
int buys_for[FAMSIZ];
 
M

Merrill & Michele

I think there exists enough information in this thread in order to take
a/* Ever C program that I will consider begins with a hash
and ends with a close brace */
#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP \
(tmp=buys_for[m],buys_for[m]=buys_for[n],buys_for[n]=tmp)
#define > ' ' /*use max for greater than,*/
int main(){

int i;
int m,n,tmp,t,top_num;
int buys_for[FAMSIZ];
/*i is always available for dummy*/
top_num=RAND_MAX - ( RAND_MAX % FAMSIZ) - 1;
srand(time(NULL));

/*initialize*/
for (i=0;i<FAMSIZ;i++) buys_for=i;

/*main control*/
 
Ad

Advertisements

M

Merrill & Michele

/* Every C program that I will consider begins with a hash
and ends with a close brace */
#include <[email protected]
#include <[email protected]
#include <[email protected]

#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP \
(tmp=buys_for[m],buys_for[m]=buys_for[n],buys_for[n]=tmp)
#define > ' ' /*use max for greater than,*/
#define @ 076 /*encoding specific*/
/*would I be surprised if this worked*/
int main(){

int i;
int m,n,tmp,t,top_num;
int buys_for[FAMSIZ];
/*i is always available for dummy*/
top_num=RAND_MAX - ( RAND_MAX % FAMSIZ) - 1;
srand(time(NULL));

/*initialize*/
for (i=0;i<FAMSIZ;i++) buys_for=i;

/*main control*/
 
M

Merrill & Michele

#define '.h>' @
#define > ' '
#define @ >
/* Every C program that I will consider begins with a hash
and ends with a close brace */
#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP \
(tmp=buys_for[m],buys_for[m]=buys_for[n],buys_for[n]=tmp) /*would I be surprised if this worked*/

int main(){

int i;
int m,n,tmp,t,top_num;
int buys_for[FAMSIZ];
/*i is always available for dummy*/
top_num=RAND_MAX - ( RAND_MAX % FAMSIZ) - 1;
srand(time(NULL));

/*initialize*/
for (i=0;i<FAMSIZ;i++) buys_for=i;

/*main control*/
printf("%d number=", t);
return 0;
}

 
M

Merrill & Michele

#define '.h>' @
#define > ' '
#define @ '.h>'
/* Every C program that I will consider begins with a hash
and ends with a close brace */
#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP \
(tmp=buys_for[m],buys_for[m]=buys_for[n],buys_for[n]=tmp)
/*would I be surprised if this worked*/
int main(){

int i;
int m,n,tmp,t,top_num;
int buys_for[FAMSIZ];

/*i is always available for dummy*/
top_num=RAND_MAX - ( RAND_MAX % FAMSIZ) - 1;
srand(time(NULL));

/*initialize*/
for (i=0;i<FAMSIZ;i++) buys_for=i;

/*main control*/

printf("%d number=", t);
return 0;
}


 
M

Merrill & Michele

#define '.h>' @
#define > ' '
#define @ '.h>'
/* Every C program that I will consider begins with a hash
and ends with a close brace */
#include <stdio.h>
#include <stdlib.h>
#include <time.h>
#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP \
(tmp=buys_for[m],buys_for[m]=buys_for[n],buys_for[n]=tmp)
/*would I be surprised if this worked*/

int main(){

int i;
int m,n,tmp,t,top_num;
int buys_for[FAMSIZ];

/*i is always available for dummy*/
top_num=RAND_MAX - ( RAND_MAX % FAMSIZ) - 1;
srand(time(NULL));

/*initialize*/
for (i=0;i<FAMSIZ;i++) buys_for=i;

/*main control*/

printf("%d number=", t);
return 0;
}



Eight strikes, I'm out. Please don't tell me you read all this way because
I was OT. That would just make you dumb. MPJ
 
K

Keith Thompson

I was not even aware that VAX C still exists as a supported HP product.

I have no idea whether it is; I suspect it isn't.

The last time I actually worked on OpenVMS, we used both VAXC (on
VAXen) and DECC (on VAXen and Alphas) (I advocated abandoning VAXC at
the time). This was about 5-6 years ago. Now I have a stray account
on an old VAX/OpenVMS system (6.3, I think) that happens to have VAXC
and DECC installed; it's likely nothing on that system is supported.
I have no interest in support for it anyway, so I haven't looked into
it.

The more general issue is this: Always pay attention to compiler
diagnostics. Fixing your code so it compiles without warnings is
usually, but not always, a good idea. The exceptions are those
(hopefully rare, assuming a good compiler) cases where you know better
than the compiler.

Fixing your code doesn't mean tweaking it until the warning goes away,
it means understanding the problem and correcting it. Adding a cast
can eliminate a type conflict warning, but it's usually better to make
the types match in the first place. Declaring "void main" can
eliminate a warning about a missing return statement, but it's better
to add the return statement (and complain to the vendor if the warning
actually advises you to use "void main", as we saw here recently in
another thread).

To summarize the summary of the summary, programming is hard work.
 
Ad

Advertisements

M

Merrill & Michele

falscher Ansatz MPJ
#define '.h>' @
#define > ' '
#define @ '.h>'
/* Every C program that I will consider begins with a hash
and ends with a close brace */
#include <stdio.h>
#include <stdlib.h>
#include <time.h>


#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP \
(tmp=buys_for[m],buys_for[m]=buys_for[n],buys_for[n]=tmp)
/*would I be surprised if this worked*/

int main(){

int i;
int m,n,tmp,t,top_num;
int buys_for[FAMSIZ];

/*i is always available for dummy*/
top_num=RAND_MAX - ( RAND_MAX % FAMSIZ) - 1;
srand(time(NULL));

/*initialize*/
for (i=0;i<FAMSIZ;i++) buys_for=i;

/*main control*/

printf("%d number=", t);
return 0;
}


Eight strikes, I'm out. Please don't tell me you read all this way because
I was OT. That would just make you dumb. MPJ
 
R

Richard Bos

Tim Rentsch said:
Acronyms that are pronounced/used as regular words eventually become just
regular words and we tend to forget their acroynm-ness. "Laser" is an
example of that, or "spool" (and in fact I used the word spool for
quite a while before finding out that it is an acronym).

Spool is probably a backronym. Even if it isn't, it was almost certainly
"designed"[1] from the start to remind one of those round, flanged
objects which are used to hold a length of thread, or a print job
created on one dinosaur, written to magtape or even punched tape which
was, well, spooled onto a spool, transferred to another dinosaur which
controls the printer, and there rolled off the spool, read and printed.

Richard

[1] scare quotes intentional
 
D

Dan Pop

In said:
2004-compliant. I have two macros. Mr. Pop's RND was not even close to
right.

I have asked you to explain why. If you are unable to do it, there is no
point in repeating it. I have given you a *complete* program. Please
point out the flaws in its results.

My RND is flawed *only* when the size of the family is very close to
RAND_MAX, or when the size of the family is a small power of two and the
actual implementation of rnd() is extremely poor (only the last bits of
the "random" value are used in this case).

If you have other objections to it, please state them loud and clear.

You may also want to elaborate on why your problem *requires* a high
quality sequence of random numbers.

Dan
 
M

Merrill & Michele

2004-compliant. I have two macros. Mr. Pop's RND was not even close to
I have asked you to explain why. If you are unable to do it, there is no
point in repeating it. I have given you a *complete* program. Please
point out the flaws in its results.

My RND is flawed *only* when the size of the family is very close to
RAND_MAX, or when the size of the family is a small power of two and the
actual implementation of rnd() is extremely poor (only the last bits of
the "random" value are used in this case).

If you have other objections to it, please state them loud and clear.

Dan, all fifteen bits count. If rand() stinks, then don't make it worse.
You may also want to elaborate on why your problem *requires* a high
quality sequence of random numbers.

Because, if it's two lines of code to get it right, then you get it right.

If interest remains after I get this darn thing put together from top to
bottom, then we can talk about this stuff. It would really help to be able
to read and write files, but I'm not there yet. I think I have to use win32
tools for files right now. Correction, right now, I have a ton of work to
do:-( MPJ
 
M

Merrill & Michele

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP \
(tmp=buys_for[m],buys_for[m]=buys_for[n],buys_for[n]=tmp)


int main(){
int i;
int m,n,tmp,t,top_num,penry;
int buys_for[FAMSIZ];

/*rand() prelims*/
top_num=RAND_MAX - ( RAND_MAX % FAMSIZ) - 1;
srand(time(NULL));

/*initialize*/
for (i = 0; i < FAMSIZ; i++) buys_for = i;

/*main control*/
for(m = 0; m < FAMSIZ; m++){
while(1){
t = rand();
if (t > top_num) continue;
penry = t % FAMSIZ;
if (penry == m) continue;
if (buys_for[m] == buys_for[penry] || buys_for[penry] == buys_for[m])
continue;
break;
}
SWAP(m,n);
}

/*out to console*/
putchar('\n');
for(i = 0; i < FAMSIZ; i++) printf ("%3d", i);
putchar('\n');
for(i = 0; i < FAMSIZ; i++) printf ("%3d", buys_for);
putchar('\n');

return 0;
}
 
Ad

Advertisements

M

Merrill & Michele

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP \
(tmp=buys_for[m],buys_for[m]=buys_for[n],buys_for[n]=tmp)


int main(){
int i;
int m,n,tmp,t,top_num;
int buys_for[FAMSIZ];

/*rand() prelims*/
top_num=RAND_MAX - ( RAND_MAX % FAMSIZ) - 1;
srand(time(NULL));

/*initialize*/
for (i = 0; i < FAMSIZ; i++) buys_for = i;

/*main control*/
for(m = 0; m < FAMSIZ; m++){
while(1){
t = rand();
if (t > top_num) continue;
n = t % FAMSIZ;
if (n == m) continue;
if (buys_for[m] == buys_for[n] || buys_for[n] == buys_for[m])
continue;
break;
}
SWAP(m,n);
}

/*out to console*/
putchar('\n');
for(i = 0; i < FAMSIZ; i++) printf ("%3d", i);
putchar('\n');
for(i = 0; i < FAMSIZ; i++) printf ("%3d", buys_for);
putchar('\n');

return 0;
}


/*one bug down*/
 
M

Merrill & Michele

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define FAMSIZ 15 /*between 2 and RAND_MAX*/
#define SWAP(m,n) \
(tmp=buys_for[m],buys_for[m]=buys_for[n],buys_for[n]=tmp)


int main(){
int i;
int m,n,tmp,t,top_num,penry;
int buys_for[FAMSIZ];

/*rand() prelims*/
top_num=RAND_MAX - ( RAND_MAX % FAMSIZ) - 1;
srand(time(NULL));

/*initialize*/
for (i = 0; i < FAMSIZ; i++) buys_for = i;

/*main control*/
for(m = 0; m < FAMSIZ; m++){
while(1){
t = rand();
if (t > top_num) continue;
penry = t % FAMSIZ;
if (penry == m) continue;
if (buys_for[m] == buys_for[penry] || buys_for[penry] == buys_for[m])
continue;
break;
}
SWAP(m,n);
}

/*out to console*/
putchar('\n');
for(i = 0; i < FAMSIZ; i++) printf ("%3d", i);
putchar('\n');
for(i = 0; i < FAMSIZ; i++) printf ("%3d", buys_for);
putchar('\n');

return 0;
}
 
T

Tim Rentsch

I disagree. Even for normal abbreviations, not just for acronyms,
industry-standard abbrevs are probably more legible than the whole word.

I mean, my main program deals with entering data for advertisement. Can
you imagine how much larger and less legible my code would've been if
I'd used advertisement_width_in_columns everywhere I know have adv_cols?

How about 'ad_columns'? My unabridged dictionary includes 'ad' as a
word. If 'ad_columns' is still too long for you, 'ad_width' has the
same number of letters as 'adv_cols'.

Ew, ganoo style.

It's hard to know how to respond to a statement that is clearly
nothing more than name calling. If you have a comment about the usage
rather than just an objection to some people who happen to use it, how
about letting us know what that is?

By the way, I've been writing return-type-on-a-separate-line style
function definitions in C since before the GNU project was formed.

generate_random_derangement( unsigned a[], unsigned n ){
unsigned i, j, t, r, r_limit;

r_limit = RAND_MAX - RAND_MAX % n;

for( i = 0; i < n; i++ ) a = i;


Ew ew ew! You just proved that whitespace isn't _always_ good (and
whitespace on the _inside_ or parens rarely is).


If indeed something has been proved here, a better candidate is that
the spacing style used above is something Richard Bos is unaccustomed
to seeing.

The spacing style exemplified in the 'for' statement above arises
out of several considerations (among others):

1. The multiple spaces make it easier for my eyes to separate
the several control clauses of a 'for', and easier for my
brain to chunkify them.

2. Following a precept of Hoare's: Things that are different
should look different; hence, I don't put spaces inside
parentheses surrounding sub-expressions (at least not
usually), but I do put spaces inside parentheses used in
function calls/definitions, and also inside parentheses
used with control expressions.

3. Control expressions (in 'for', 'if', 'while', etc) are
different in character from other expression uses, and
often more central to understanding how an algorithm
works; using two spaces around control expressions helps
them stand out from the other code around them, which in
my experience aids comprehension.

Aside from your gut reaction, do you have any statement to offer about
why one might prefer an alternative spacing style over a style like
the one shown above? It seems like all you've said is, "In my
opinion, it's bad." Anything more objective to propose?

How about this more
moderate version:

for (i = 0; i < n; i++) a = i;

In fact, I'd prefer

for (i=0; i<n; i++) a=i;

But double spaces? Yuck.


I've tried both of the moderate versions. I prefer a style that is a
little more heterogeneous, for the reasons explained above; just as
in writing prose, too much homogeneity can itself make the text less
readable.

I admit the wider spacing looks strange at first and takes some
getting used to. That doesn't have to mean it's the wrong path;
the first time I ate sushi I didn't like it either.
 
Ad

Advertisements

T

Tim Rentsch

Tim Rentsch said:
Acronyms that are pronounced/used as regular words eventually become just
regular words and we tend to forget their acroynm-ness. "Laser" is an
example of that, or "spool" (and in fact I used the word spool for
quite a while before finding out that it is an acronym).

Spool is probably a backronym. Even if it isn't, it was almost certainly
"designed"[1] from the start to remind one of those round, flanged
objects which are used to hold a length of thread, or a print job
created on one dinosaur, written to magtape or even punched tape which
was, well, spooled onto a spool, transferred to another dinosaur which
controls the printer, and there rolled off the spool, read and printed.

Yes, spool was a bad example. Even if it wasn't "designed", certainly
the word spool was a word before the acronym came into existence.

On the other hand that doesn't invalidate the point. We still have
radar, sonar, and scuba, for example.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Top