3
\$\begingroup\$

I'm didn't use java for a long time(5 years ± ) and in the following code the user should provide yaml file and it converting it to json file.

The code is working as expected but since I didn't use java for a long time I want to see if I can write this code better and learn new things :-) in the latest java (currently with use of java8 not 9)

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;

import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.util.Scanner;
import java.util.concurrent.atomic.AtomicReference;


public class yaml2json {


    public static void main(String[] args) {


        //Get File Path
        String sourceFilePath = getSourceFilePath(args);

        //Load File content
        AtomicReference<String> content = new AtomicReference<String>();
        try {
            content.set(readYamlFile(sourceFilePath));
        } catch (IOException e) {
            e.printStackTrace();
        }

        //Json string
        String jsonStr = null;
        try {
            jsonStr = convertYamlToJson(content.toString());
        } catch (IOException e) {
            e.printStackTrace();
        }

        //Print json file
        sourceFilePath = sourceFilePath.substring(0, sourceFilePath.length() - 3);
        sourceFilePath = sourceFilePath + "json";

        try {
            FileWriter fileWriter = new FileWriter(sourceFilePath);
            BufferedWriter bufferedWriter = new BufferedWriter(fileWriter);
            bufferedWriter.write(jsonStr);
            bufferedWriter.close();
            System.out.println("Success " + sourceFilePath);
        } catch (IOException err) {
            System.out.println("Error writing to file: " + err );
        }

    }

     private static String readYamlFile(String pathname) throws IOException {

        File file = new File(pathname);
        StringBuilder fileContents = new StringBuilder((int)file.length());
        Scanner scanner = new Scanner(file);
        String lineSeparator = System.getProperty("line.separator");

        try {
            while(scanner.hasNextLine()) {
                fileContents.append(scanner.nextLine() + lineSeparator);
            }
            return fileContents.toString();
        } finally {
            scanner.close();
        }
    }


    private static String getSourceFilePath(String[] args) {
        String inFile;
        if (args.length != 0) {
            inFile   = args[0];
        }else {
            inFile = "/Users/rayn/IdeaProjects/myproj/a.yml";

        }
        return inFile;
    }


    private static String convertYamlToJson(String yaml) throws IOException {

        ObjectMapper yamlReader = new ObjectMapper(new YAMLFactory());
        Object obj = yamlReader.readValue(yaml, Object.class);

        ObjectMapper jsonWriter = new ObjectMapper();
        return jsonWriter.writeValueAsString(obj);
    }


}
\$\endgroup\$
  • \$\begingroup\$ I see lots of view :) , if my code looks good also let me know...Thanks! \$\endgroup\$ – Rayn D Aug 28 '17 at 6:16
2
\$\begingroup\$

Simplify File Reading

Java 7 and 8 make I/O a lot simpler. You can simplify your reading of a String from your input file to this:

    private static String readYamlFile(String pathname) throws IOException {
        String lineSeparator = System.getProperty("line.separator");
        List<String> lines = Files.readAllLines(Paths.get(pathname));

        return lines.stream().collect(Collectors.joining(lineSeparator));
    }

Remove the first two Try...Catch block

Your first try...catch blocks catch IOException and then just print out the stack trace. This adds lots of uneccesarry lines for little benefit. If you add throws IOException to public static void main(String args[]) the code becomes clearer, such as:

 public static void main(String[] args) throws IOException {

    //Get File Path
    String sourceFilePath = getSourceFilePath(args);

    //Load File content
    AtomicReference<String> content = new AtomicReference<String>();
    content.set(readYamlFile(sourceFilePath));
    //... and so on

Remove the AtomicReference

The AtomicReference might be useful if the code were multi-threaded. But it's not, the content variable is local to main(). So it just needs to be a String and with the removal of the try..catch above it can be simplified to:

String content = readYamlFile(sourceFilePath);

Don't Use sourceFilePath for your outputFilePath

It's a misleading name, give the output file its own variable. You can also just do it in a single line:

String outputFilePath = sourceFilePath.substring(0, sourceFilePath.length() - 3) + 
"json";

Also - this assumes the extension is three characters, so won't work correctly with a .yaml file. You can use a library like apache-commons to do this in a more foolproof way.

String outputFilePath = FilenameUtils.removeExtension(sourceFilePath)
         + ".json";

Simplify File Writing With PrintWriter

Again later versions of Java make dealing with I/O more straight-forward. I've kept the try...catch block so you can dispay the error message. You can also use the try-with resource to auto-close your PrintWriter if anything goes wrong

    try (PrintWriter out = new PrintWriter(outputFilePath)) {
        out.print(jsonStr);
        System.out.println("Success " + sourceFilePath);
    } catch (IOException err) {
        System.out.println("Error writing to file: " + err );
    }
\$\endgroup\$
  • \$\begingroup\$ HI Thanks! , few short question :) 1 . at the end why you keep the try catch block 2. for a string like "json" should I create Constant ? 3. regard the 'FilenameUtils' it's not overkill to add the dependency of Apache just for it ? \$\endgroup\$ – Rayn D Aug 28 '17 at 13:31
  • \$\begingroup\$ in addition the finally doesn't recognises the out in your example, is it true ? \$\endgroup\$ – Rayn D Aug 28 '17 at 13:34
  • \$\begingroup\$ 1. I only kept the try...catch block to maintain existing behaviour (you print out the user-friendly message, rather than just print the stack trace). You could also remove that, although the output would be uglier. 2). You could do and that would make it clearer if you wanted to change it in future but I don't think it's a big deal as it's only used once \$\endgroup\$ – matt freake Aug 29 '17 at 10:42
  • \$\begingroup\$ 3) You're 100% right - I'm used to projects with lots of dependencies. But it would still be better to look for the last dot, for instance like stackoverflow.com/a/941280/1168884 \$\endgroup\$ – matt freake Aug 29 '17 at 10:43
  • \$\begingroup\$ 4) Also correct - it was a cut-and-paste error, but it highlighted that try-with resource there would be nicer (finally block is added automatically) \$\endgroup\$ – matt freake Aug 29 '17 at 10:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.