OK, here's some initial observations, please don't feel I'm digging at you - you did ask for comments! Also bear in mind that these are just
my opinions. Others may have wildly different ideas. Please note that these are just a selection of comments , not a complete full review (I can't really afford the time for that) - at least it should give you something to be going on with.
1. Site design:
+ Lose the "under construction" it's a sure sign of a web newbie. Professional web designers know in their bellies that a web site is
always under construction and just make sure that what is there is consistent and of high quality.
+ You really have to think about the look and feel of the sites. The Magic shop in particular has a different style for every page which is very offputting. Choose a single background, font and layout style for all the pages in a site so a user feels "at home". The Angelfire front page is very "busy" and could do with a bit of consistency betwen sections. Surf a lot, and study the sites you like.
+ You are serving the site froma slow connection, so please rethink the use of those great big graphics. Particularly the store with those big strips of blurry room and no text to help you decide what it's about.
+ Your Angelfire front page seems to have some sort of animation on it which causes netscape to gobble up sysyem resources and slow my machine right down. This is not a good idea. Think twice, or three times, before including any animations or client-side dynamic content, particularly on a front page. Something like this will just drive people away.
+ You really need some sort of always-available site navigation. It's really hard to tell where you are in the sites and what else might be available. Look at any of the high-traffic sites and you'll see page headers which indicate where you are, and buttons to get you straight to other useful pages.
2. Shopping:
+ I kept getting "no database entry" for lots of the things I chose ("+1 dagger", for example). It's ok to have items not "in stock", but please give a different message. Users don't need or want to know about technical problems.
+ It's very hard to shop when the price for the items is not shown with the item name. Please consider displaying more information with the items in each section - at least price and number in stock, but consider other things like a description, delivery time etc.
+ Your shopping cart refers to "the left pane". It's much better to give the pane a heading (such as "Category List") and refer to it by name. This allows you to move your panes about later without having to change the text.
3.
Java general points:
+ Please consider making your source code available as a jar file. It's not only quicker to download, but preserves filenames and extensions, and is much simpler for someone to grab (one right-click/save-as/choose-name as opposed to 10).
+ I guess I'm more used to my own style, and to the Java Ranch style which is fairly close. I find that your Java code seems very "bunched up". Please add some blank lines to help visually separate sections such as member variables, methods etc.
3.1 ServletUtilities.java
+ I assume most or all of this is unmodified from the original. However I would prefer a more descriptive name than "filter". This is normally known as htmlEncode, and has a partner htmlDecode which does the converse.
3.2 FileIO.java
+ You don't need the "extends Object", it's the default.
+ Comments without useful content are worse than no comments at all. If you are going to use JavaDoc, then fill in all the fields and make sure they are correct and up to date, otherwise just don't bother. Also, please reconsider comments which just express something built in to Java and familiar to all reasonably experienced programmers. Lose the comment on the constructor.
+ It's generally considered bad practice to declare more than one variable or member in a single statement as it limits readability and future flexibility for no benefit.
+ The member variables "string", "line" and "file" should not be members, as they are only used within specific methods. Declare them where they are used.
+ Again, think hard about names. The names you have chose for the FileIO class and its methods are not particularly obvious. How would a user guess that one difference between "append" and "write" is that "append" adds a newline ?
+ As I look through this class I do feel that you have got a lot more code here than is necessary. You don't need a BufferedWriter if you are just writing one string and then closing the file, and are you aware that all Writers have a "write(String)" method so you don't need to create a PrintWriter just to do a "print"? Study the standard APIs.
+ This class would fail a "Pure Java" check as it uses a hard-coded newline character rather than using System.getProperty("line.separator")
+ Your "write" and "append" methods are quite similar, consider refactoring them to separate common code (XP.s "Once And Only Once" (OAOO), or the Pragmatic Programmers' "Don't Repeat Yourself" (DRY) principle).
+ It's generally considered a bad thing to build up Strings using "+", especially in loops as this causes a
lot of new Strings to be created then abandoned. Using a StringBuffer is a much better choice.
To summarise, here's a comparison between the original and a suggested example rewritten version incorporating the above points:
ORIGINAL CODE:
EXAMPLE REWRITE
3.3 CheckoutServlet.java
+ bunched up, members should be local variables etc. as above.
+ I'm really worried that this servlet is SingleThreadModel. This is a really bad idea unless there is some sort of external resource you are accessing which is only single tasking.
You should never use member variables in a servlet, for this reason.
+ Your "doPost" method is far too big. It tries to do too much. Split it up into calls to methods each of no more than 10 lines or so, and consider splitting some of it out into separate classes.
+ You have hard-coded the database details. These really should be externally specified (servlet init parameters are a good choice, as is JNDI, property files or whatever). I'd also recommend that you look into connection pools, either as code that you incorporate into your application, or provided by the servlet container. Managing database connections in a servlet is both inconventient and slow.
+ Putting all your code in your "doPost" method also has some subtle side effects. If you can't load the database driver, you just return from doPost without sending anything to the outputStream - this will cause an unsightly 500 error. If all that processing was in a separate method, you could still just return from it, but the main "doPost" could still continue to write out some sort of user-friendly error message.
+ I'm also worried by the somewhat messy mixture of Java and HTML. This says to me that you should be separating the two somehow, wither by converting all the database processing to a bean and wrapping it in a
JSP, or by using a templating mechanism such as WebMacro, FreeMarker or XSLT to fill an external template with your values.
+ It's almost always good practice in a servlet to provide handlers for both doGet and doPost, even if one is just a call to the other. You never know when you might want to pass a form using GET in
testing to see the values in the URL, for example.
3.4 DatabaseServlet.java
+ lots of comments as for CheckoutServlet.
+ Ugh! cut-n-paste code! The database connection code in this servlet is the same as in CheckoutServlet. You should
never do this, particularly with a piece of code like this which includes so many things which might change. Ask yourself "how many places would I need to change and recompile if I changed the ODBC DSN or the databse driver or the database?". If the answer is more than one (hint, it is...) then your code is much less maintainable than it should be. Get this stuff out of your servlets and into a connection pool or at least into another class used by all the servlets. Let "Once And Only Once" become your mantra, and keep refactoring your code until you get there.
3.5 GetUserInfoServlet.java
+ all the usual comments.
+ Remember that getParameter() can return null, so don't risk NullPointerExceptions. My top tip is to always put the constant string first in an "equals" if you have one; don't say "if (name.equals(""))", say "if ("".equals(name))".
3.6 NameServlet.java, MyCookieServlet.java
+ all the usual comments
3.7 StartingBalanceServlet.java
+ I don't think most people have a "Calligrapher" font. Please at least provide an alternative, but ideally rethink why you need it at all.
I hope the above gave you something to think about. Now let's see what everyone else has got to say ...