File editing program

Question

I have written a Java class to replace text in file. The file contains database configurations and is as follows:

test.properties
#local
db.url=jdbc:oracle:thin:@localhost:test
#db.user=testa
#db.password=testa
db.user=testb
db.password=testb
#db.user=testc
#db.password=testc

# is a comment character. Now db schema testb is in use.

Here is my Java class to switch between schemas

import java.io.BufferedReader;
import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.FileReader;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
public class SwitchTo
{
 public static void main(String[] args)
 {
 Map<String, String> schemaMap = new HashMap<String, String>();
 schemaMap.put("a", "testa");
 schemaMap.put("b", "testb");
 schemaMap.put("c", "testc");
 String newText = "";
 for (int i = 0; i < args.length; i++)
 {
 newText = args[i];
 }
 String newdb = "";
 newdb = schemaMap.get(newText);
 if (newdb == null || (newdb != null && newdb.length() <= 0))
 {
 System.out.println("Schema not available, please select : ");
 for (Map.Entry<String, String> entry : schemaMap.entrySet())
 {
 System.out.println(entry.getKey() + " for " + entry.getValue());
 }
 return;
 }
 try
 {
 BufferedReader file = new BufferedReader(new FileReader("test.properties"));
 String total = "";
 String line = "";
 while ((line = file.readLine()) != null)
 {
 if (!line.startsWith("#") && line.contains("db.user"))
 {
 line = "#" + line;
 }
 if (!line.startsWith("#") && line.contains("db.password"))
 {
 line = "#" + line;
 }
 if (line.startsWith("#") && line.contains(newdb))
 {
 line = line.substring(1);
 }
 total = total + line + "\n";
 }
 FileOutputStream File = new FileOutputStream("test.properties");
 File.write(total.getBytes());
 System.out.println("Switched to schema " + schemaMap.get(newText));
 }
 catch (FileNotFoundException e)
 {
 e.printStackTrace();
 }
 catch (IOException e)
 {
 e.printStackTrace();
 }
 }
}

and I have created a batch file to run this program from command line which is as follows :

switch.bat
java SwitchTo %1

Now when write at command line

c:\> switch a

Then it will comment testb in the file and testa will be uncommented.

Please suggest other improvement for this code.

1. Is using map for storing schemas is a good choice?
2. How can I make it generic to accommodate newly added schemas without need to compile this class again?
3. Is there any better algorithm for solving this?

• 200_success: Rather than editing a file programmatically, consider keeping several configuration files, and copying the appropriate one into place to activate it. It's much simpler that way!

Answer 1

Actually, this can be more generic.

Instead of hard-coding your map; fill it from your properties file.
So change your prop file to :

#db.id=a
#db.user=testa
#db.password=testa
db.id=b
db.user=testb
db.password=testb

Fill your map like this :

private Map<String, String> schemaMap = new HashMap<String, String>();
public void fillMap() throws FileNotFoundException, IOException {
 String line;
 String key = null;
 String value = null;
 try (BufferedReader file = new BufferedReader(new FileReader("test.properties"))){
 while ((line = file.readLine()) != null) {
 if (line.contains("db.id")) {
 key = line.substring(line.indexOf('='));// could be you have to do an + 1
 }
 if (line.contains("db.user")) {
 value = line.substring(line.indexOf('='));
 }
 if (key!=null && value != null) {
 schemaMap.put(key, value);
 key = null;
 value = null;
 }
 }
 } catch (IOException ex) {
 // report with Logger or System.out what happend.
 }
}

Secondly : In your main you use the args.
You do not check for how many args you have, you just take the last arg.
Maybe a check for how many args you have is appropriate here, and make the program do what you want then. (like quitting with error message or taking default arg).

Third : Always close your resources(reader/writer in this case).
Here it is done with a Try-with-resources available from java 7, otherwise you will need a finally block where you close it in.

Answer 2

Naming
You should give your classes and variable meaningful names.

Class names should not be verbs, but rather nouns - SwitchTo also doesn't really convey what the class does - DatabaseConfigurationSwitcher will do a better job.

Variable names like file, total and newText might be technically accurate, but they don't help the reader know what they mean - currentPropertiesFileContents, newPropertiesFileContents and selectedUser would better convey meaning, and will make your code a lot easier to read.

You should always abide by the naming conventions of you language of choice - camelCase for java variables - newdb is a bad variable name. File is worse.

There is no need to repeat the type of the container in its name: schemas is better than schemaMap.

Redundant code
You found a very curious way to assign the newText variable - you are actually assigning it <num of args> + 1 times! There is no need to assign it with "", and definitely no need to override it arg.length times a simple:

String newText = args[args.length - 1];

would do. The only reason I can think of which might have driven you to this strange code is your fear that args will be empty, in which case you can add:

String newText = (args == null || args.length == 0) ? "" : args[args.length - 1];

Of course, as @chillworld suggested, both are wrong, as you should check that there is exactly one argument so:

if (args.length != 1) throw new Exception("Give only one argument");
String newText = args[0];

Assumptions
For some reason, you assume that the username and password are always equal, and known at compile time. If that is the case, you could have made you life easier and instead of commenting and un-commenting lines in your code you could simply re-write the section, since you know its values:

test.properties
#local
db.url=jdbc:oracle:thin:@localhost:test
#### overwrite everything below this line with db.user=user;db.password=user ####
db.user=testa
db.password=testa

Of course, a better solution would be not to assume that you know either the username or the password at compile time, and use an external repository to hold this information (even the properties file itself could be it, as @chillworld suggested).

String concatenation
Using a String variable to collect all the lines in your code is not a good idea, as it results in a lot of new objects being built. A StringBuilder is a better type to do that, but in your situation.

Close your resources
You open a file reader, and then a file writer, but you never close them! This is very problematic, since you leave open file handles which may result in a serious memory leak. It is true that java will close all open handles when the process exits, but you should never count on that!

Don't catch what you can't handle
You have a try block, and two catch clauses, which do nothing but output the exception to the console.
A better way would be to declare main(String[] args) throws FileNotFoundException, IOException, and not even write the try block.
This way, in case of an error, not only will its stack trace be printed in the console (via the error stream), but the console will also be notifies that the command ended abnormally, and the command users will be able to refer to that (looking at the errorlevel).

Answer 3

I suggest to add some meta-information in comments as follows:

#local
db.url=jdbc:oracle:thin:@localhost:test
#begin_schema: a
#db.user=testa
#db.password=testa
#end_schema
#other unchangeable properties
some.property.a=value1
some.property.b=value2
#begin_schema: b
db.user=testb
db.password=testb
#end_schema

so, the comments #begin_schema/#end_schema will demarcates the boundary of blocks for facilitation of parse and process.

here is my solution:

import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
public class SchemaSwitcher {
 static final Pattern BEGIN_PATTERN = Pattern.compile("^#*\\s*begin_schema:\\s*(.+)\\s*$");
 static final Pattern END_PATTERN = Pattern.compile("^#*\\s*end_schema.*$");
 static enum State {NO_OP, ADD_COMMENT, REM_COMMENT}
 public static void main(String[] args) throws Exception {
 String fileName = args[0];
 String schema = args[1];
 Path path = Paths.get(fileName);
 Files.write(path, switchSchema(Files.readAllLines(path), schema));
 }
 static List<String> switchSchema(List<String> lines, String schema) {
 State state = State.NO_OP;
 for (int i = 0; i < lines.size(); i++) {
 String line = lines.get(i);
 switch (state) {
 case NO_OP:
 Matcher matcher = BEGIN_PATTERN.matcher(line);
 if (matcher.matches()) {
 state = schema.equals(matcher.group(1)) ? State.REM_COMMENT : State.ADD_COMMENT;
 }
 break;
 case ADD_COMMENT:
 if (END_PATTERN.matcher(line).matches()) {
 state = State.NO_OP;
 } else {
 if (!line.startsWith("#")) line = "#" + line;
 }
 break;
 case REM_COMMENT:
 if (END_PATTERN.matcher(line).matches()) {
 state = State.NO_OP;
 } else {
 while (line.startsWith("#")) line = line.substring(1);
 }
 break;
 }
 lines.set(i, line);
 }
 return lines;
 }
}

• Vogel612: Adding a #begin section and #end section are a bit overkill for 2 properties, that are "always correctly formatted". If it were a more complex config-file, I'd really love your suggestions. btw. #end section is unneccesary, if you assume, that a section cannot contain subsections... Still +1
• twester: closing tag (end_schema) allows to have unchangable block between two shcemas. if you think that this is redundant, it's very easy to change the code to consider the closing tag as optional

Answer 4

Addition to the answer of @chillworld and @Jamal

use java.nio instead of java.io

private Map<String, String> schemaMap = new HashMap<>();
public void fillMap() throws IOException {
 String key = null;
 String value = null;
 Path path = Paths.get("test.properties");
 try {
 List<String> lines = Files.readAllLines(path, Charset.defaultCharset());
 for (String line : lines) {
 if (line.contains("db.id")) {
 key = line.substring(line.indexOf('='));// could be you have to do an + 1
 }
 if (line.contains("db.user")) {
 value = line.substring(line.indexOf('='));
 }
 if (key != null && value != null) {
 schemaMap.put(key, value);
 key = null;
 value = null;
 }
 }
 } catch (IOException e) {
 // report with Logger or System.out what happend.
 }
}