Showing posts with label AbstractASTNumber. Show all posts
Showing posts with label AbstractASTNumber. Show all posts

Monday, June 16, 2014

If Your Only Tool Is a Hammer Then Everything Looks Like a Nail

While I was working on the project in the past week a few things of note came up that are worthy of mention. At great time-expense, a mentor was gracious enough to do a thorough code-review. The review fished out a lot of unaddressed problems lurking within the code, chief amongst which was the apparent sparseness of the various set and get methods inside the library. For the get methods I was neglecting to throw a PropertyUndefinedError [ I was unaware that such errors could be thrown in the place of primitives, go figure ] on the event that the requested entity is nonexistent and no NullPointerException should be thrown. Similarly, all the set methods were missing the proper listener notifications. While I had done this for a few of the set methods, the importance of it all had been somewhat lost on me. It is only when I did my own modest investigations and became privy to the internal workings of the library that the gravity of the task crystallized in my mind and I set about implementing the required modifications.

The presence of these problems in the first place is perhaps best explained by the title of this entry. This far into the project, the current ASTNode class, has been my primary reference. All the features I have implemented have been informed by whatever implementation is currently present in the ASTNode class. Thus, it never occurred to me to question any of the code I was mimicking (especially when the code's purpose was unclear, as is generally the case given how monolithic ASTNode is). This whole ordeal, has just reminded me of the importance of always being skeptical and keeping an open mind when examining other people's code.

There were also a few issues with inconsistency, misleading javadocs and innocuous problems of a similar quality (i.e. not a pressing issue, but still worth looking at). With the help of my mentors I compiled a list of all the major problems plaguing the app and sought to address them over the latter half of the week. I believe that I have been largely successful. As the new week commences there's only a few ends I need to tie up. The new features that I've added include, (1) Adding a proper clone method to each class. (2) Adding proper constructors to every class. (3) Using Eclipse to automatically generate class-specific toString() methods that implement string builders (4) Lay the framework for a new compiler ... etc

Another issue that was raised that as of right now remains unaddressed, is the issue of which classes to keep abstract. After a code review led by one of the mentors it came to our collective attention that the ASTNumber and ASTFunction classes really didn't have to be regular classes, since what they (both) represent is somewhat generalized and amorphous. During regular use of the library it is unlikely that users will find any need to create instances of either class directly, thus making their constructors unnecessary.

Finally, duplication within the class hierarchy, as exemplified by the ASTCSymbol group of nodes, has been a contentious area, with more than one person citing unnecessary duplication and calling for a better way of representing the implied relationships. As with the ASTNumber / ASTFunction the final solution will have to be a collective decision. This is why I have been hesitant in implementing a solution (even a tentative one).

The next non-JSBML entry I post will focus on Eclipse. Eclipse is powerful, and is pretty much the IDE of choice for Java developers. Eclipse's popularity is due. in large part, to its impressive and easily tractable plugin system (implemented with OSGi) as well it's large store of features, many of which, are directed towards enhancing productivity for power users. These so-called 'productivity-boosters' will be the main focus of the next entry. Hopefully, they will be able to spur some sort of discussion on the topic.