Long term efforts to improve the codebase

[tuxayo]

It's more a discussion about what should be the priorities to keep the codebase maintainable and how can we improve it incrementally.

Duplication between modules

Whether it's in the database or in the code itself, having now everything in the same repo allows to share stuff.

For example it seems that generic classes like DBService or other abstractions are 99% the same between modules. So for example when finishing the conversion from mysql to mysqli in Management, it would have been useful to study the possibility of merging the DB related classes with another module that was already converted. Instead of doing the same work and contributing to make them diverge in meaningless ways.

Coding style

• tabs in most places, spaces in some and even mixes of them (see Coding guidelines: chose an indentation style #55)
  • alignment are often broken (in different ways depending of the editor settings!)
  • difficulties to match related blocks (if/else, especially when there is nesting) which leads to comprehension errors
• trailing whitespaces
• windows end of lines in some files
• inconsistent spacing
  • while/if (condition) { vs while/if (condition){
  • } else { vs }else{

For reasons difficult to explain, among a non negligible share of developers, these can create a lot of frustration and distract from getting work done.

One strategy that I'm experimenting is before making non trivial changes to a file (and having to deal with these style issues) I do a quick pass to fix the most blatant issues once for all in a separate commit.

For now it seems to work well.

Eliminating notices and warnings

When using Coral each click usually generates dozens of them and spams the logs with undefined variables, indexes, offsets, etc. Making difficult to spot one's owns mistakes, hiding existing bugs, making investigations more difficult.
Example of the noise added when investigating an error and how it can worry the users (ops in the libraries) which end up disabling the error logging.

It seems that there are not too many different causes and therefore gradually adding simple checks would allow to shut these messages.

Anything that could be done better? Because these strategies are a bit of improvisation given the low experience with Coral and PHP that I have.

What are the other things that we could do to gradually improve the code and make Coral more fun to develop?

[jsavell]

Adopt a true MVC pattern.

Right now, our data classes are sprinkled with view and business logic and our controllers and views are merged into one entity. This greatly complicates modifying code or adding new features.

We should work toward using interfaces to define contracts for interacting with our data models.

In other words, code that works with an Organization implementation shouldn't have to know or care if it's representing a org module Organization, a resources module Organization, or importable organization data from a 3rd party API.

Our CSS needs attention.

It's ad hoc, and not easily reusable or extensible. Part of this is due to the over-reliance on IDs serving as jquery hooks (which also tightly couples our HTML to our javascript to our CSS, making it difficult to improve any of the three). Another part is the abuse of the !important declaration, which breaks the C in CSS and is difficult to identify and fix.

[jsavell]

The Javascript needs to be refactored.

• There's been a tendency to copy and paste, rather than abstract, which means what should be trivial changes end up being hours spent making sure the simple change is implemented in many, nearly identical places.
• Because of the tangled JS, we're stuck using a very old version of jQuery, which lacks many new features, performance enhancements, and security fixes.

Improve Coral's Modularity

• I think this is well known and being addressed, but the lack of shared code across modules means innovations in one module often don't make it to others, meaning we're actually dealing with several codebases.
• Coral is flexible in its ability to function in combination with different modules, but all code that hinges on expressing a given configuration should be in as few places as possible, preferably one.
• In the resources module, for example, the module logic is peppered throughout class methods and controller files. Right now, a developer just has to learn through trial and error when they need to account for the fact that an organization could be native, or from the organization module.

[jsavell]

To supplement the interface point, here's an example of a feature we're working on at TAMU to import record data from an in house API:
TAMULib@2449358

Because there's a well defined interface for what an Organization is, the controlling code doesn't need to know what type of Organization implementations it's working with. It just knows getTitleText() will be callable. It doesn't care if that method is backed by a MYSQL query or an API request.

[tuxayo]

Updated initial message part about the error messages with an example of real life issue.