You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Sep 30, 2021. It is now read-only.
Some general comments, tip, and suggestions for this lab:
2
4
3
-
# Correctness
5
+
##Correctness
4
6
5
7
The one non-subtle issue that came up in several places was that
6
8
people simply failed to free temporary memory they'd allocated.
7
9
`valgrind` is very good about catching these kinds of problems, so
8
10
there's really no reason for not finding and fixing those problems.
9
11
10
-
There was a strong inverse correlation between length and
11
-
correctness. The broken `array_merge` submissions were almost all
12
-
well onto the second page, where the correct ones almost all fit
13
-
nicely on a single page. This may be partly due to the increased
14
-
tendency of broken programs to have print code and commented code,
15
-
but probably more generally reflects a lack of clarity.
16
-
17
12
Several people got lost in their array indices, and some remained
18
13
lost there throughout. This is a nice example of a problem where
19
14
loop invariants could be really useful in helping you stay clear
@@ -37,30 +32,11 @@ Several groups had off-by-one errors where they weren't allocating
37
32
you need to make sure that you allocate *N+1* characters so there's
38
33
room for the null terminator at the end. This can be tricky to find
39
34
because the tests actually pass even if you make this make this
40
-
mistake. (To be honest, I'm not entirely sure *why* the tests pass.
41
-
I assume that somehow we're getting lucky and getting a 0 byte at
42
-
the end of our strings. I suspect that if we dug around some we
43
-
could come up with a test that would fail, but I haven't been able
44
-
to do so yet.) `valgrind` picks it up, though, and complains about
35
+
mistake. `valgrind` notices this problem and complains about
45
36
illegal reads and writes when we try to access the memory location
46
37
where the null terminator would need to be, but which we never
47
38
allocated space for.
48
39
49
-
A number of groups had a subtle mistake where they
50
-
51
-
1. Dynamically allocated an array that was potentially empty: `int *a = calloc(n, sizeof(int));`.
52
-
1. Copied some data into that array using a loop that (correctly) did nothing if *n=0*.
53
-
1. Accessed the first item in the array, which might not actually be there, e.g., `int i = a[0];`.
54
-
1. Protected the remainder of the code (through an `if` or loop with appropriate bounds) so that the value taken from the array was never *used* if n=0.
55
-
56
-
This is broken since the line `int i = a[0];` is accessing a
57
-
memory location that hasn't been allocated to your program. It's
58
-
probably a legal memory address, though, so it's unlikely to actually
59
-
*fail*. And since the subsequent guards ensure that you never actually
60
-
use the (probably random) value that you got back, there probably aren't
61
-
any tests that will actually catch the mistake. Luckily `valgrind` catches
62
-
it, complaining about an illegal read.
63
-
64
40
# Style and clarity: The big issues
65
41
66
42
Please use decent variable names! C is a challenging language and
@@ -75,20 +51,9 @@ Initialize variables where you use them. Several people declared
75
51
between. (Breaking things up into smaller functions would do much to
76
52
lessen this problem - see below.)
77
53
78
-
Use functions to break things up! Many of the submissions have
79
-
`array_merge` as a single function that went on for over a page. Several people have a
80
-
second function that looks for duplicates, which is nice, but that
81
-
was about it by way of modularity for most folks. Remember the bit
82
-
about C being a challenging language, and not making it worse?
83
-
Applies here, too. A nice rule of thumb (for any language) is that a
84
-
function should have at most one loop. Almost everyone had at least
85
-
three loops in their `array_merge` function, with several people
86
-
getting as high as 6. I'd have a look at that if I were you.
87
-
88
-
On a related note, *many* people have their vowel check in
89
-
`disemvowel` as a huge long chain of `||` statements. Pull that
90
-
stuff out into a named function, *especially* if you end up
91
-
repeating it!
54
+
Use functions to break things up! In the past, for example, many people
55
+
have had their vowel check in `disemvowel` as one huge long chain of `||` statements. Pull that stuff out into a named function, *especially* if
56
+
you end up repeating it!
92
57
93
58
# Style and clarity: Odds and ends
94
59
@@ -107,20 +72,10 @@ Don't `#include` `.c` files; you want to compile `.c` files
107
72
linker pulls them all together into a working executable.
108
73
109
74
Pay attention to warnings, and ask if you don't understand what it's
110
-
saying. There were several cases where the compiler warnings pointed
111
-
right to the problem.
75
+
saying. It's not uncommon for people to ask questions when the
76
+
compiler warnings are pointing right to the problem.
112
77
113
-
Use C's `bool` type (with values `true` and `false`) instead of `int`s
78
+
As appropriate, use C's `bool` type (with values `true` and `false`)
79
+
instead of `int`s
114
80
with the values 0 and 1. Booleans are a lot more readable and less error
115
81
prone.
116
-
117
-
# Algorithmic issues
118
-
119
-
Several people did a variant of linear search to see if an item was
120
-
duplicated. Use the fact that your array is sorted (or go ahead and
121
-
sort it if it isn't) to simplify this (and speed it up, but the
122
-
simplicity is arguably the bigger issue).
123
-
124
-
---
125
-
126
-
These notes were started by Vincent Borchardt, 16 Aug 2012
0 commit comments