In general, we give 20 points for code quality, taking points off for violations under the three broad categories:
As above, a good name can help your code be self-documenting. Naming a function something like helper or doStuff is of really no help to anyone. Examples in class or discussion typically use meaningless names like foo and bar - these are good when the functions we're describing really have no meaning other than to illustrate a point, but in your code, there should be no such functions.
Copy/paste code is one of the strongest indicators that you should create a new function, or that two branches could be unified.
It's easy to give examples of over-commenting, e.g.:
Bad:
but it's much harder to give examples of under-commenting, except by absurdity (zero comments for your entire assignment). Usually you don't need to comment what you've done ("add one to x"), but why you've done it ("move pointer beyond the last path separator we found in order to prepare to process the next argument"). If you think it would be confusing to someone who doesn't know the system, you probably should comment it.
// Add one to x
x++;
For example, struct_ptr->field is common, (*struct_ptr).field looks a little jarring.
Most of the time, there's no reason to use the comma operator.
Copying a struct field-by-field is unnecessary, confusing, and vulnerable to breaking the system in hard-to-detect ways if the struct definition ever changes. Two accepted ways are *struct_ptr = new_struct_value or memcpy( struct_ptr, &new_struct_value, sizeof( struct type ) ).
*(arr + i) is almost always less clear than arr[i]. The C language lets you play fast-and-loose with types system (e.g., declare an array of bytes but treat groups of these bytes as struct inodes), but you shouldn't when you can avoid it. Declaring a buffer of a generic type and casting it later encourages code like *((struct my_type *)arr + i), which is more obnoxious to read and error-prone. Declaring the buffer of the type you meant it to be anyway is probably a better decision.
If there are two equivalent ways of doing the same thing (e.g., structPtr->field vs. (*structPtr).field or arr[i] vs. *(arr + i)), learn or ask which is preferred, and use that one. Figuring this out typically requires reading a lot of both good and bad C code. If in doubt, search the internet, ask on the forum, or talk to us during office hours.
The C standard library handles many common tasks (searching, sorting, processing strings, basic math). Often, there are functions for manipulating strings, arrays, etc. that you would find useful. You might attempt to implement them yourself, but the C library functions will always be better optimized and have gone through far more hours of debugging than yours ever will.
In order to learn the C library, either search the internet for the intended effect, or ask in office hours or on the discussion forum for suggestions. Read function specifications very carefully - some are easier to use than others (e.g., strtok has side-effects on the given argument, most other string manipulation routines do not, strdup creates memory on the heap, most other string manipulation routines do not).
Unnecessary casts clutter code. First, see the above note about C idioms. Second, learn when it is appropriate to rely on the compiler to automatically perform the necessary cast. For example, if you're relying on certain integer widths or doing arithmetic with mixed integer and floating point types, it might be appropriate to add a cast for readability or accuracy. If you're passing a pointer to a function that expects a generic type (usually void *), a cast adds clutter. For example, memcpy( (char *)buf, (char *)buf2, BUF_SIZE ); adds nothing - the compiler will cast the underlying types to void * anyway, and the use of it next to the size might give the mistaken impression that memcpy is taking type into account.
malloc/free isn't worth it. If you do opt to use heap memory, make sure you free it when your program is no longer using it.
Don't copy large chunks of memory when its unnecessary. Be careful with input and output parameters to functions. Pay attention to pointer 'levels', e.g., don't use char ** when you mean char *.
Ideally, your code should have no raw numbers in it other than 0 and 1 (and if you think about it, 0 and 1 are often better expressed in other ways, e.g., ++, NULL, or implicitly in a conditional). Typically, if your code is depending on some value that might change depending on architecture, hardware, or new requirements, but probably won't change for months, years, or even generations of architecture, that value should be #define'd or declared a const global. This is especially true of values that might change depending on several issues, e.g., number of blocks in an indirect block.
Another way to think about this: If you're tempted to put a comment explaining why a number appears in your code, you're better off replacing that number and comment with a #define or const. The variable name itself can help serve as documentation, moving you closer to the ideal of self-documenting code.
// BAD
// Coyote needs to cross the river five times...
for( int x = 0; x < 5; x++ ) {
...
}
// GOOD
#define COYOTE_RIVER_CROSSINGS 5
for( int crossing = 0; crossing < COYOTE_RIVER_CROSSINGS; crossing++ ) {
...
}
Functions should be "the right length". For all of the code in CS110, that length is typically going to be very short (probably 10-30 lines on average, likely with more short functions than long functions). If you're writing functions that are much longer than the average function we're giving you, think about why. Guidelines below might help clarify this.
Functions should have simple type signatures. If a function really needs more than four or five arguments, think about whether or not it makes sense to group those arguments into a single struct or to split the function into two or more smaller functions.
The main reason to add a new function is because a piece of code can be reused. Another reason to add a new function is to clarify the intent of the code you're running. A well-named function can simplify high-level understanding of an algorithm while also providing the benfits of a comment. If you're tempted to add a comment to the top of a block of code, would it make more sense to refactor that block into it's own function, named using the information that would have been contained in the comment?
Of course, keep in mind there is an overhead associated with function calls (versus inlining the code). Sometimes this overhead becomes significant in comparison to the work done in a function. However, unless performance becomes an actual issue, prefer clarity to efficiency.
Pay attention to the abstractions, and make use of them. For example, consider the concept of "layer bypass" in assignment 1. Read section 2.5 of the book carefully, considering which layer is meant to communicate with which other layer. Consider whether or not a given layer actually has to be aware of another layer. If a given layer does not have to be aware of another layer, ensure that it is not.
If you don't intend a line to be run, remove it before submitting. This doesn't mean comment it out, unless as a comment you believe it useful to future understanding, this means delete it.
Use whitespace; think of it as writing paragraphs. Different TAs might use different editors, so you want to make your code as generally readable as possible. Accordingly, keep your lines short, consistently use either spaces or tabs (this might involve a quick find/replace over the code we give you), and leave an extra line of whitespace in logical places to break up otherwise monolithic blocks.
Back