HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Civilization 5 mod validator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
validatormodcivilization

Problem

I would like to submit a portion of my code for Code Review, and I have done my best to comment the code (and include essential parts that may aid understanding of the program) for your convenience! Any optimisation that would increase the speed of the running of the program would be appreciated!

The program is designed to check through Civilization 5 mods to ensure that they have not make mistakes and finds mistakes that users would have to load the mod (which takes a long time compared with the amount of bugs that can be present) to discover. My program performs this task much faster than the conventional method and so will be useful for modders like myself to quickly check their mods.

First a little housekeeping:

```
private static Vector statements = new Vector (); //all the requires-where added by the program or the user live here
private static Vector baseGameStatements = new Vector (); //when the base game files are run through this system, any errors are put here, and ignored later on

/*
* This is the sort of format for the requires-where statement
* taking the first requires as an example:
the program requires that in the "register" there is at least two Tags with identifiers that match ANY CIVILIZATION.
* (This is important, CIVILIZATION_ENGLAND and CIVILIZATION_FRANCE must both be present twice in the following tables)
* in the Tables with an identifier of:
* Civilization_UnitClassOverrides
* Civilization_BuildingClassOverrides
* Improvements
* the required tag can be present in any of those tables,
* so one CIVILIZATION in Improvements and another in Civilization_UnitClassOverrides would qualify the two required
*/
public static void reset()
{
addRequires(2, "(Civilization.ClassOverrides)|Improvements", "CIVILIZATION.", false);
addRequires(10, "Civilization_SpyNames", "CIVILIZATION.*", false);
addRequires(1, "Civilization_CityNames", "CIVILIZATION.*", false);
addRequires(1, "Civilization_Leaders", "CIVILIZA

Solution

First, some basics:

-
Vector is a class that is deprecated.... actually, not deprecated, but discouraged....


As of the Java 2 platform v1.2, this class was retrofitted to implement the List interface, making it a member of the Java Collections Framework. Unlike the new collection implementations, Vector is synchronized. If a thread-safe implementation is not needed, it is recommended to use ArrayList in place of Vector.

and, if you need synchronization, which you do not, then you should be looking at java.util.concurrent.* classes anyway.

-
Code Style: Java Code Style recommends putting the opening brace { on the same line as the condition/statement for if/for/while structures....

-
If you are using String.format(...) anyway... you may as well replace:

tableNames += tableNames.isEmpty() ? String.format("%s", name) : String.format(" or %s", name);


with:

tableNames = String.format("%s%s%s", tableNames, tableNames.isEmpty() ? "" : " or ", name);


but using a StringBuilder will be a good option too.

With those small things out of the way .... and, by the way, the change from Vector is not really 'small', it has a big performance impact.... then you can look at the bigger items.

-
Function extraction - break out your inner loops in to discrete functions. This has a number of performance benefits too, primarily related to how the Java code is compiled when it is used often.

-
Use for (Map.Entry ... : map.entrySet()) {...} loops, instead of map.values() and map.keySet()

In all, I would also wonder whether it is an option to keep things at the XML level, and use something like Saxon, or JDOM (which I maintain) as a mechanism for running XPath expressions on. Saxon has the ability to index the documents, and XPath expressions end up being very efficient if the document is big, and static... which this may be. The Saxon index will be more efficient than what you have (or it should be).

Code Snippets

tableNames += tableNames.isEmpty() ? String.format("%s", name) : String.format(" or %s", name);
tableNames = String.format("%s%s%s", tableNames, tableNames.isEmpty() ? "" : " or ", name);

Context

StackExchange Code Review Q#47789, answer score: 8

Revisions (0)

No revisions yet.