Coding Standards

Coding Standards

If the Moyoman project is to be successful, eventually millions of lines of code will be written. Therefore, it is crucial that all code which is submitted follow certain guidelines. Code which is submitted will not be accepted unless it meets these standards, so make sure that you understand them before you start work. Some of these items, such as formatting, apply to all of the code, but most of them apply to the code under org.moyoman.module or org.moyoman.helper.

Formatting

This is the least important. It doesn't really matter if you use tabs or spaces, or if you put the { on the same line or next line as the if statement. If you are writing a new package, then you may use any reasonable coding standard. If you are submitting code to an existing package, try to follow the conventions for that package so that the new code is similar to the existing code.

Do not make assumptions about particular implementations of a module or helper.

In managing the huge volume of code that we are hoping to have, it is crucial that modules and helpers be loosely coupled. The way that this is done is to use well defined interfaces. Thus, a life and death module may make use of a shape module and a groups module. It should not know or care which implementations of the Shape or Groups interface it is actually using, but just use the ones passed to it by the makeMove() or generateMove() methods. It certainly should not instantiate them directly. The analogous statements are true for helpers.

Implement the Module abstract methods, and interface methods exactly.

There may be a temptation to have additional public methods to the ones required by the Module class, and the interface being implemented. Don't do this. If the interface definition is not finalized, propose adding the additional methods, but don't add any public methods that are not in the Module class or the interface to your implementation. They cannot be used without the user assuming that they are using your particular implementation. This is not allowed, as explained above. The crucial thing is to make sure that the interfaces are defined correctly. Analogous statements are true for helper implementations.

No back doors.

The module or helper implementation should be interacting with the system by using the well defined interfaces. When the makeMove() method is called, the internal state is updated. When generateMove() is called, analysis is done to determine the next move. There is almost never a reason for a module to open a socket to another machine, send email, etc. Check before using packages such as java.net to make sure that this is really necessary.

Use the correct classes and methods to get your work done.

Because of certain Java limitations relative to C++, i.e, no friend keyword, it was not possible to break the framework code up into different packages and still ensure the correct visibility of classes and methods at compile time. For example, the org.moyoman.log.SystemLog.error() method should not be called directly, but rather the Module.error() or Helper.error() method should be used. There are a number of other examples of this, listed below. The rationale for each of these decisions is also explained below.

Use equals() instead of ==

There are a number of classes in the util package, such as Color, Point, and Stone, that limit the total number of objects; 3 for Color, 361 for Point, and 1083 for Stone. Because of this, you might be tempted to write code like this:
if (st1 == st2)
and if the color, x, and y coordinates of the two Stone objects are the same, expect that this statement is true. Unfortunately, there is a problem. If a game is saved and then reloaded, the total number of objects of these classes has increased. So, for this code sample:
Color c1 = Color.BLACK;
Color c2 = Color.BLACK;
if (c1 == c2)
The if statement is not necessarily true if c1 was created by the running process, and c2 was created during a previous session of the program, serialized by the save command, and then reloaded. The equals() method will return true and so should be used instead of ==. There may be a way to override the deserialization operation to ensure that the total number of objects does not increase, but investigating this will be put off to some future time. So, use
if (st1.equals(st2)) {}
if (c1.equals(c2)) {}
instead.

Fail silently

One of the design goals of a module is that it always produces some output, if at all possible. Of course, a joseki module which cannot read its dictionary may not have much choice, but it is preferable to produce the best output possible in spite of any errors and not throw an exception. The error should be logged by calling error(), and then can be fixed in the future. This allows the current game to proceed if possible.

Be careful in implementing clone()

The clone() method in Java is a fertile source of bugs. Since the Moyoman framework uses cloning extensively, any bugs in the clone() method will have an impact on the program. For example, if you have a HashSet variable in your module, there are three ways of handling it. The first is to ignore it, in which case the two objects will be sharing the HashSet object. The second is to add the statement:
mod.hs1 = (HashSet) hs1.clone();
This causes there to be a different HashSet object for the original and cloned module, but the objects in the HashSet are shared. The third is to add the statements:
mod.hs1 = (HashSet) hs1.clone();
Iterator it = hs1.iterator();
while (it.hasNext())
   mod.hs1.add(it.next().clone());
This causes the original and cloned module to each have its own HashSet object, and the objects that each contains are unique to that object. It will almost always be the case that the second or third way will be correct. Make sure that you understand this, and the implications of the approach you use.

Be careful with various Collection classes

This point is related to the one about cloning. You might find yourself retrieving a value from a HashMap, where the key is a certain type of object that you have created as part of your module package. If the key was cloned, then your retrieval might fail. You might be able to solve this problem based on overriding equals() and hashCode(), but that might not always be appropriate.

An example of a way of dealing with problem can be seen by looking at the org.moyoman.util.closestpoint.ClosestPoints.refresh() method. Basically, if the key is from a different cloned object than the HashMap, if you can determine a mapping between the cloned key and the key from the correct object, then you get the correct key first, then do the retrieval from the HashMap.

Another type of problem has to do with derived classes. If a HashMap in your module used Point objects as a key and some type of module specific object as the value, for your public methods that access the HashMap, you need to make sure that you are really using Point objects. For example, it would be perfectly reasonable for the caller to pass a NumberedStone object in instead, and expect to retrieve the value for the corresponding Point object. This is the reason for the Point.castToPoint() and Stone.castToStone() methods. Make sure that you code defensively, and make these calls before performing the lookup or storage on your HashMap.

Be careful overriding the equals() and hashCode() methods

One way of dealing with problems mentioned above with cloning and Collection classes is to override the equals() and hashCode() method of your classes. For example, if an object is a collection of Stone objects, the hashCode() method could be based on that set of objects. That way, a retrieval using your object as a key would work even if the key had been cloned. The problem with this is that it no longer works if you add or remove a Stone from your object. In this case, trying to be clever by overriding these two methods will just lead to trouble.

It is considered to be bad style to have o1.equals(o2) return a different result than o2.equals(o1). For this reason, the range of hashCode() return values for Point, Stone, and NumberedStone objects are all different, and so explicit conversion from the derived class to the base class are required for the purpose of accessing a Collection object.

The point of this coding standard is not to specify the "correct" way of handling this problem; it is to make sure that you are aware of the problem to make design and debugging of your module easier.