[csu540-f05-rpf] General and student-specific comments on SP1

[Robert Futrelle]

The comments below should help you with SP2, coming up
in just over 48 hours as I write this.

  -- Prof. Futrelle
------------------------------------------------------------

CSU540 Fall 2005 Programming Assignment SP1
Semester Project #1 - Cones
Professor Futrelle's comments on the assignments as handed in

Below are some general comments on SP1, followed by
specific comments I sent to individual students.
The comments are anonymized to omit any references
to specific individuals.

---------------------------------------
One person used 100 faces on the cone - very smooth appearing model.
Another with many faces had little gaps, which lead to strong
aliasing patterns.

Many people didn't transform their cones to "interesting" angles;
just had them horizontal or vertical.

Javadoc:  Comment class fields too.
   People are still not paying attention to the -package command line option.
   Hardly anyone used <p> or <br> to split lines.
   One person used red text to highlight a note, i.e.,
    <font color="red">Note: </font>

Learn how to get Javadoc to document package private classes and fields too.

Clean up your documentation so it reflects what your project is about.
Don't let it contain leftovers from prior projects that don't make sense
in the new context.

Some defined to many similar fields, e.g., for vertices.
Should keep such collections in arrays or similar data structs.

Ambient light so weak for some that portions of the cone
   were black. Ambient is there to avoid that.

Some chose one color for the light, e.g., red, and another
for reflectivity, e.g., blue.  The net result is black.
So think more carefully about the colors you choose.
Normally the reflectivity is a single value, but the
ambient and point light sources can reasonably be chosen
somewhat differently.

In using Color, many people converted integer values by scaling with 1/255.
Why people didn't use 0.0 to 1.0 everywhere, I I don't understand.

I realized that a trivial way to create the second cone would be
to copy the first one, a clone of a sort, and then rotate one of them
around the x or y axis by pi radians, 180 degrees.  No worry then
about the order of the vertices - they have to remain correct.

In screenshots with text comments, you could put your name,
date, course, etc., somewhere such as at the bottom of your screen.
Then it would be more self-documenting. Some did this.

Essentially everyone left untouched some of my old javadoc in
RTDrawererthat includes the phrase "Draws a rectangle". You all really
should have edited that to fit what *you* were doing. Even someone who
added their own name to the comment left the rectangle comment in.

In each triangle, the vertices could be stored as an array of  Vecs,
   rather than in separate fields.

I haven't found anyone yet who maintained their three colors in an array.
   Everyone used separate fields or local vars for the three components.
In both these cases, iterating over these arrays is easier than ever,
   in Java 5, an example I found:

This example sums the number of items in an array of Products:

public int totalQuantity(Product[] products)
{
     int total = 0;
     for(Product p : products) {
         total += p.getQuantity();
     }
     return total;
}

Little use of toString() to look into your object instances,
   especially for the Tri class.

In String, concatenate with +.  If one of the items being concatenated
is an object, its toString() is automatically invoked, e.g.,
String s = "My frog model values are " + frog1;
No need to call toString() explicitly in this case.

---------------------------------------
Specific student comments:
---------------------------------------

You left all your backup source file copies in your space - messy.
Model a good addition to just one cone.
You can and should javadoc your class fields too.
Could have used a color vectors[] instead of three named items everywhere.
You don't need "this" most places.  Fields are visible inside methods.
You don't need to define the default constructor.
Your Readme doesn't say what the program is about, what it does.
You say, "Light at 100,100" (2D? light *vector*?)
Screenshots very good, but your ambient lighting too weak.

---------------------------------------

You used solid fill, not Gouraud. The latter involves averaging
normals and interpolating.
Your Readme was fine.
Your cone was rather small.

---------------------------------------

No need to have "Cone class" or "Model class" as javadoc comments;
   javadoc does that for you.
No need to call toString() explicitly. It's done for you
   whenever a print statement finds and object embedded in a string.
Your private method comments never appeared in the Javadoc.
Could have used a color vectors[] instead of three named items everywhere.
Your ambient light didn't seem to work, since parts of your cones
   appear to be absolute black, even though your code seems to have
   ambient light included.
Your Readme doesn't say what the program is about, what it does.
Lots of screenshots and command line options -- good.

---------------------------------------

Your default Cone constructor comment never appeared in your javadoc,
   because it was only package visible.
Use ambient on every triangle, don't test to see if incident is zero;
   the textbook doesn't test; just adds ambient everywhere.
TinyRayTrace is not in your javadoc and still has some planet comments in it.
Did you ever use drawTri() - easy way to get started, no tracing required
   except that it could draw occluded tris - does all in order, I'd guess.
No screenshots, I never saw any rotations.
When I run your code, the object doesn't look like a pair of cones.
Also seems to be no ambient light.
Reasonable job - not super.

---------------------------------------

Good looking screenshots - seem to have ambient working.
You commented package visible fields - good.
Using Java 5, I see. (Next big code work I do will go to 5 too.)
Could have defined toString() for the Tri class,
   nicer prints in main() test then.

---------------------------------------

You used toString() - good.
Using 100 faces a good idea.
Good javadoc.
No "interesting" angles in shots.

---------------------------------------

Your Readme had no name, date, class, etc.
an anonymous document.
You had no rotated screenshot - maybe only translated?
You seem to have more in your code than you showed in your shots.
Did your checker code work - no shot?

---------------------------------------

Interesting aliasing phenomena showed up because you used many triangles.
Not a problem, just interesting.
Too many distinct fields defined; could have kept them as vectors, e.g.,
vectors of vertices.
RTDrawer doc says it draws a rectangle - clean up your javadoc.

---------------------------------------

Screenshots good, esp.  demo1.
Worth naming even your top-level package so it won't be "<Unamed>".
paintComponent() doc still mentions a rectangle.
Don't need to call toString explicitly. You could have included
some text from a toString() output.
All in all, a good job.