Technical documentation

Hi Sylvain,

I see that you have already done some work on coding. Before I make any alterations, I want to know what does what and how you intended to structurize the classes and code. What I also miss are some simple things as coding standards. You probably started working on the code in Java 7 style, and in the pom.xml you specify that the coding standard is 1.8, however I do not see any code using the language features of Java 8. There are a lot of public methods, but hardly any of them are documented, so it is quite hard to understand what you intend to achieve with them. Without any "contracts" it will be hard to keep the work going forward.

Could you provide some architectural documentation with this project?

Kind regards,

Cor

Hey Cor, I can provide a document with architectural details, no worries. I will create it in the wiki so you can have access to view and edit. To make it short, the code is split is 3 sections, interpreter, statements and compiler. The interpreter parses the SDFL source code and transforms the text files into statement objects (ex.: InsertIntoStatement or ImportStatement, etc.) with the help of a statement builder. Once the statements are loaded, they are passed to an implementation (Oracle for now) of the SDFL compiler. It compiles the statement objects into proprietary SQL code using a SQLCodeGenerator.

Interpreter https://github.com/fragmatyc/SDFL/blob/master/SimpleDataFixLanguage/src/com/sdfl/interpreter/SDFLInterpreter.java

Statement https://github.com/fragmatyc/SDFL/blob/master/SimpleDataFixLanguage/src/com/sdfl/statements/impl/InsertIntoStatement.java

Statement Builder https://github.com/fragmatyc/SDFL/blob/master/SimpleDataFixLanguage/src/com/sdfl/statements/builders/impl/InsertIntoStatementBuilder.java

Oracle Compiler https://github.com/fragmatyc/SDFL/blob/master/SimpleDataFixLanguage/src/com/sdfl/compiler/impl/oracle/OracleSDFLCompiler.java

Code Generator https://github.com/fragmatyc/SDFL/blob/master/SimpleDataFixLanguage/src/com/sdfl/compiler/impl/oracle/sql/statement/OracleInsertIntoStatementSQLCodeGenerator.java

The project is done in TDD and all tests are passing.

Hi Cor, Don't worry about the programming standards, I will make them tomorrow on the wiki. Just make sure you have a look and tell me if you don't agree with any of them. As you said, the main bases are done. All we need to do is implement the different compilers/interpreters and statements. Once you got it, it should be straightforward. Have fun!

Development Standards: https://github.com/fragmatyc/SDFL/wiki/Development-Standards I'm working on the architectural documentation.

coding standard is 1.8, however I do not see any code using the language features of Java 8. You're right. I started off with 1.7 (1.8 was not out). The use of the JDK8 comes with much more improvements than Lambda expression or streams, if that's what you meant by features of Java 8. If you think that changing the actual code's for-each to streams' forEach() will improve readability, maintainability and/or performance, we should log a ticket to modernize the code.

We shouldn't go for Lambda expressions and streams just for the sake of those. Unless you'd want to experiment with them (I have created a local branch to just play around with them), we should at least provide some rules when to use them. Streams for instance give a great base for parallelism, however if your collection is only ever going to be parallellizable in 2% of the cases there is not much gain. Sometimes streams will even slow your application down. forEach() is now part of the Collection<> interface, so streams are not necessary here per se either.

Lambdas can be an improvement, but not always. Again caveat emptor.

Right-o, I just had some time to read the development standards.

They are pretty common sense overall, which is good. Although I do have some questions:

You specify a OO programming acronym STOLID. I've try to find it, but came no further than a bird on wikipedia (https://en.wikipedia.org/wiki/Stolid_flycatcher). I did however find SOLID, my guess is that STOLID is a typo.
You specify that all tests must start with "testThat". What is the thought behind this? All tests already have an annotation @Test, so we already know that it is a test. That the name of the test should express the intention of the test and the expected outcome I can agree with, spending 8 characters which realistically don't really add anything is just introducing white noise in my opinion.
There are no development standards regarding the formatting of the code. When working solo on a project that is no problem. However as soon as you start inviting people over to work on the code, you will notice that there are a huge number of variations on how to format the code and as soon as somebody is going to use a 'code prettyfier' you will notice it in your merge actions. I would have expected some rules (perhaps in the form of an Eclipse or Idea formatting.xml) regarding whether to use tabs or blanks for indentation etc. I would like to advice you to setup these kinds of rules and to place the rules in IDE readable form somewhere in the project.
Naming conventions. I recognize the pattern of using a prefix for the scoping of the parameter, I think it was called Hungarian notation and this seems to be a variation. Whilst this standard made sense in the late 1980s and early 1990s, with today's IDE tools there is not much added value. What are you trying to achieve with this standard? In my view it obfuscates the actual name (it hinders me). If anything I would suggest that you stated that the name of the variable should express the intent and/or usage of the variable. So no rather anonymous "id" but the more expressive "customerId", the latter expressing the semantically more correct contents of the variable (now everybody knows it is a customer, not a vendor or article id.).
What is your view on programming to interfaces? I personally rather like to program against interfaces rather than concrete implementations and using factory patterns to obtain concrete implementations. Not only helps this with testing (you can easily mock an interface), but you could switch implementations quite easily. But how does one name the concrete implementations? One convention (which I abhore) is Impl. Yes, it is an implementation (that is what Impl is trying to convey) but what specific implementation is it? Is it the default, is it the implementation when connected or something like that? My suggestion is to not have the name end on 'Impl' but to have it express the conditions under which the implementation works or what it does.
You say that all classes must have " the comments header (License)", but what is that header? All my classes have a header, they don't have something called License but if you want the word License in it, fine by me. My guess is that you have a common class header in mind, if you do: provide that header as a template. Then everybody can copy that into her/his IDE and use that as a template.
Avoiding all comments is a bit harsh. I think that, allthough we should indeed strive for clear and expressive code, concise and to the point comments can really help. One example is for instance to clarify the thought behind a construct or a reference to a used pattern - especially if that pattern is relatively unknown. Having to jump back and forth between test and implementation hinders developers and may even frustrate them. Yes keep the comments to the point and short, but I think forbidding them is not done. Also with javadoc; especially javadoc is useful when expressing the contract for a method. For interfaces it defines the behaviour which can be expected from implementations, for public methods it defines the behaviour which can be expected or the way that method works (without having to read the code). Yes, there is a risk of out of date documentation here, but that is why you keep the documentation short and concise.

Regarding the architecture: Printing out the javadoc of classes is just scraping the surface of architecture. I would have expected a bit more in the sense of setting out the way in which components interact and standards regarding external frameworks allowed (or not). For instance: if somebody wants to introduce a framework such as Spring, Apache.lang, Apache.io, Hibernate or Guice, is that allowed? And why not? Architecture is also about setting the playing field and specifying the leeway of the developers, architecture is also about specifying how modifications need to be delivered (e.g. for each issue assigned to me I make a separate branch which I create a separate pull-request for; others may directly try to resolve it in their master branch, others may create a patch to be pulled in and merged).

What is your view on exceptions? Do we only use java-provided exceptions or do we have our own exceptions? Do we have a mix (java-default as much as possible and our own when non is available) and what is the naming convention here?

Cor, Sorry for the delay! I'll try my best to answer all your points:

The T in STOLID refers to "Tell, don't ask" as the SOLID principles seem to be lacking a word for encapsulation. "Tell, don't ask" says you must avoid asking an object for it's state before doing some logic operations with it.
The intention behind the prefix "testThat" is to write a complete sentence. We currently have 104 tests that all start with "testThat". I'm aware of the Given When Then, but we'll keep those for functional tests.
Regarding the development standards, I'm surprised you don't know there are code conventions developed by Sun in the early days of Java. By default, your IDE should format to those conventions. If there are any recurring problems during pull requests, we should integrate Checkstyle. I will refer you to this link: http://www.oracle.com/technetwork/java/codeconventions-150003.pdf
Instance variables are prefixed with "this" (always), class constants are in capital letters with underscores instead of spaces, local variables are prefixed with "l" and parameters with "p". You can configure your IDE to add them automatically.
Regarding interfaces, use your judgement. If we respect the principles of point 1 and develop using TDD everything should be fine. But, please, do not create interfaces all the time. If you cannot find a good name other than ClassSomethingImpl, that means it's overkill. For example, ImportInputFileLoaderFileSystemImpl is an implementation of ImportInputFileLoader that targets the file system. The Impl postfix is superfluous, I agree. ImportInputFileLoaderFileSystem would be enough.
Regarding the header, I was talking about file header (before package) and not Class javadoc. Hopefully, I have not missed one, but all files should actually be starting with the same header.

/**
 * This file is part of the Simple Data Fix Language (SDFL) core.
 * 
 * All components of the language (compiler, interpreter, etc.) are
 * free and open source: you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation, either version 3 of the License, or
 * any later version.
 *
 * SDFL is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with SDFL.  If not, see .
 * 
 * @author 
 */

Regarding the comments, avoid them at all cost. If your interface method's name is not self-explanatory, and the implementation requires more that 5 lines of code, then it's bad and need refactoring. Don't worry, I know it's not realistic! Just keep the above rule in mind, but don't overembellish, it should be fine. If you need comments or doc, simply just don't write any business logic to avoid disconnection between code and comments :)
Regarding the architecture, everybody should be able to suggest improvements, add libraries, etc. No worries on that, we do not have to stick to a particular structure as long as we modularize the compiler implementations (easy to add new).
Another point from you paragraph is regarding collaboration. Perhaps we should create a section destined to collaboration in the README.md file and also add a new COLLABORATING.md file? Feel free to propose any suggestions!
For exceptions, we have ParsingException and CompilingException. if you look in the package com.sdfl.exceptions, you will see there are a bunch of them. All exception thrown should be one of ours, none of them should be checked exceptions.

Again, this is a work in progress. Feel free to propose anything that won't change the actual code base.

fragmatyc / SDFL

Technical documentation #2