Password storage slim/trim it down?

Question

I have created a personal password storage program that takes the user's username, password, and for what account it is for, and stores it in a text file and can also read from them (assuming you type the right name in). What I'm looking to do, is either trim it down (take out none needed code) or finding more effective ways to go about this.

import java.io.*;
import java.util.*;

public class testing_file
{
    public static void main(String args[])
    {
        Scanner input = new Scanner(System.in);

        System.out.println("Access or create?");
        String select = input.nextLine();

        switch(select)
        {
            case "Create":

                String name, user, password;
                FileOutputStream fop = null;
                File file;
                System.out.println("What account is this for?");
                name = input.nextLine();
                System.out.println("Enter the Username");
                user = input.nextLine();

                try
                {
                    PrintWriter pw = new PrintWriter(new BufferedWriter(new FileWriter("c:/"+name+".dnx")));
                    file = new File("c:/"+name+".dnx");
                    fop = new FileOutputStream(file);

                    if (!file.exists())
                    {
                        file.createNewFile();
                    }

                    System.out.println("You may now input your password");
                    password = input.nextLine();

                    pw.write("Account: " + name);
                    pw.println();
                    pw.write("Username: " + user);
                    pw.println();
                    pw.write("Password: " + password);
                    pw.flush();
                    pw.close();
                    System.out.println("Operation Complete");

                }
                catch (IOException e)
                {
                    e.printStackTrace();
                }
                finally
                {
                    try
                    {
                        if (fop != null)
                        {
                            fop.close();
                        }
                    }
                    catch (IOException e)
                    {
                        e.printStackTrace();
                    }
                }
                break;

            case "Access":
                String string="";
                System.out.println("Enter the account you wish to view");
                name = input.nextLine();
                String file1 = "c:/"+name+".dnx";

                //reading
                try{
                    InputStream ips=new FileInputStream("C:/"+name+".dnx"); 
                    InputStreamReader ipsr=new InputStreamReader(ips);
                    BufferedReader br=new BufferedReader(ipsr);
                    String line;
                    while ((line=br.readLine())!=null)
                    {
                        System.out.println(line);
                        string+=line+"\n";
                    }
                    br.close();
                }
                catch (Exception e){
                    System.out.println(e.toString());
                }
                break;

            default:
                System.out.println("Invalid method, terminating program");
                break;
        }
    }
}

Answer 1

Couple of Points.

Most Important: If your looking for secure password storage do not write your own library. Password storage is a very complex at best pratice. So I you care about the contents of the plain text password file being accessed don't do this.

If this is a learning exercise or not that critical go ahead, just keep that in mind.

Now for the actual review.

• First thing that grabs me is if you are using Java 7 then you should look at using try with resources. Try with resources has two advantages for your code. First it always closes the streams (Under normal jvm operation) secondly it is a little less verbose to write.

Basic syntax would be :

        try (PrintWriter pw = new PrintWriter(new BufferedWriter(
                new FileWriter("c:/" + name + ".dnx")));)
        {
        //Rest of code here.
        catch (IOException e)
        {
            e.printStackTrace();
        }

This eliminates the need for a lot of the closing resources.

• Second problem you open a File reference that is never used. Remove all occurences of fop and file.
• Consider moving the two "cases" to seperate methods. They are seperate things and don't need to share blocks.
• I'm not convinced as your printing the stack trace anyway if there is any need to catch the exceptions. Let the method just throw them (Note this is only because this is a self contained program I'm suggesting this.

EDIT:

This is a first stab at refactoring. I've followed my advise above then further more there are three places where your writing a key and value into a stream followed by a blank line. I've moved that to a seperate method. I've also added in your pattern for user input to a method. Hopefully you find this shorter and not too hard to read.

There are a few other points that came up when I was doing this but their not so important.

• Class Name: Your class doesn't follow java naming conventions.
• All in Main: If the objective is to practice with java please note that this isn't very object orientated. The standard java idiom would be to create a class (possibly an instance of itself) and set the values you want on this for example no need to pass scanner and pw arround as I have done in my refactor.

• Naming no huge deal but you should consider more meaningful names for variables "string" I'm looking at you.

  import java.io.*;
  import java.util.*;
  
  public class testing_file {
      public static void main(String args[]) throws IOException {
  
          Scanner input = new Scanner(System.in);
          String select = prompt(input, "Access or create?");
  
          switch (select) {
              case "Create":
                  createPasswordEntry(input);
                  break;
  
              case "Access":
                  readPasswordFile(input);
                  break;
  
              default:
                  System.out.println("Invalid method, terminating program");
                  break;
          }
      }
  
      private static String prompt(Scanner input, String question) {
          System.out.println(question);
          return input.nextLine();
      }
  
      private static void readPasswordFile(Scanner input) throws IOException {
          String name = prompt(input, "Enter the account you wish to view");
  
          //reading
          try (
                  InputStream ips = new FileInputStream("C:/" + name + ".dnx");
                  InputStreamReader ipsr = new InputStreamReader(ips);
                  BufferedReader br = new BufferedReader(ipsr);
          ) {
              String line, string = "";
              while ((line = br.readLine()) != null) {
                  System.out.println(line);
                  string += line + "\n";
              }
          }
      }
  
      private static void createPasswordEntry(Scanner input) throws IOException {
          String name, user, password;
          name = prompt(input, "What account is this for?");
          user = prompt(input, "Enter the Username");
  
          try (
                  PrintWriter pw = new PrintWriter(new BufferedWriter(new FileWriter("c:/" + name + ".dnx")));
          ) {
              password = prompt(input, "You may now input your password");
              writeKeyValue(pw, "Account", name);
              writeKeyValue(pw, "Username: ", user);
              writeKeyValue(pw, "Password", password);
              pw.flush();
              pw.close();
              System.out.println("Operation Complete");
          }
      }
  
      private static void writeKeyValue(PrintWriter pw, String key, String name) {
          pw.write(key + ": " + name);
          pw.println();
      }
  }