Learning by translating - Follow the Rubberduck - Part 2: Beta

This project is my learning place for a few things:

• MVP (model view presenter)

• XML (parsing, editing and leveraging)

• deeper swing functionality

Concerning the XML part I have already recieved a very nice review by rolfl on my previous question. Since then quite a lot of things have changed and the current state of the code is available on github

I implemented a few features, the most significant change since back then may be the free choice of translated locale. In addition to that I now support an "Unsaved Changes" dialog upon closing.

Furthermore I have removed interfaces that have only a single implementation (so basically, all), except for the OverviewView, which I want to implement with a different UI provider than swing.

Enter the Translation Helper.

As Entry point serves your trusty Main-Class:

```
public class Main {
public static final String RUBBERDUCK_PATH = "RetailCoder.VBE/UI";
public static final String ARGUMENT_MISMATCH = "Arguments do not match up. Please provide one single path to read the Rubberduck resx from";
public static final String ILLEGAL_FOLDER = "Rubberduck .resx files can only be found under RetailCoder.VBE/UI. Please give a path that points to a Rubberduck UI folder";

private Main() {
}

public static void main(final String[] args) {
// parsing the first argument given into a proper path to load the resx
// from
if (args.length != 1 && args.length != 3) {
// don't even bother!
System.out.println(ARGUMENT_MISMATCH);
return;
}
Path resxFolder = Paths.get(args[0]);
// normalize path to allow checking
resxFolder = resxFolder.normalize();

if (!resxFolder.endsWith(RUBBERDUCK_PATH)) {
System.out.println(ILLEGAL_FOLDER);
return;
}

TranslationPresenter tp = new TranslationPresenter();
OverviewModel m = new OverviewModel();
OverviewView v = new Swing

public static final String RUBBERDUCK_PATH = "RetailCoder.VBE/UI";

This value shouldn't be compiled into the binary. It should be a configurable setting that's modifiable without needing to recompile.

"RetailCoder.VBE" is a relic of the original project, before it was even called "Rubberduck", when the Mug formerly known as @retailcoder was mucking around with the VBE API. The project has had 2 if not 3 major architecture changes in the past, nothing says it's not going to have another in the future: this RUBBERDUCK_PATH value is coupled with a project structure you have no control over, that you can't assume isn't going to ever change in the future.

The ILLEGAL_FOLDER message is also dependent on the RUBBERDUCK_PATH value:

public static final String ILLEGAL_FOLDER = "Rubberduck .resx files can only be found under RetailCoder.VBE/UI. Please give a path that points to a Rubberduck UI folder";

Why not just concatenate RUBBERDUCK_PATH into the message instead of making 2 places for it to change?

The FILENAME_REGEX works now:

private static final String FILENAME_REGEX = "^.*RubberduckUI\\.?([a-z]{2})?\\.resx$";

That will support a filename like RubberduckUI.de.resx, but Rubberduck could very well be tweaked one day, to support more localized translations - and a filename like Rubberduck.de-CH.resx or Rubberduck.de-AT wouldn't match the regex, despite being valid. Of course there shouldn't be much need to make a whole new translation for a localized translation.. but we can't know that - especially if/when we get into languages like zh-CN or zh-TW, which may require completely different wordings and tokens.

In other words:

private String fileNameString(final String locale) {

It's not a locale, it's really the language - the format being language-LOCALE, each being a 2-character code. Referring to the language code with "locale" might get confusing if/when localized translations are supported.

StackExchange Code Review Q#105831, answer score: 10