Simple text DataBase

Question

So I made this simple text database system for storing a list of objects because I hate using other database programs. But I was just wondering what you guys think I could do to make this faster and less resource intensive.

package com.gmail.welsar55.WDB;


import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.io.PrintWriter;
import java.util.ArrayList;

public class Managing {

    public static String mime = "wtdb";

    public static boolean addObject(File file, Object object) throws IOException
    {
        if(file.getName().endsWith(mime))
        {
            PrintWriter pw = new PrintWriter(new BufferedWriter(new FileWriter(file, true)));
            pw.println(object.toString());
            pw.close();
            return true;
        }
        else
        {
            return false;
        }
    }

    public static boolean containsObject(File file, Object object) throws IOException
    {
        if(file.getName().endsWith(mime))
        {
            BufferedReader br = new BufferedReader(new FileReader(file));
            boolean IsGood = false;
            while(br.ready())
            {
                String line = br.readLine();
                if(line.trim().equals(object.toString()))
                {
                    IsGood = true;
                }
            }
            br.close();
            return IsGood;      
        }
        else
        {
            return false;
        }
    }

    public static boolean removeObject(File file, Object object) throws IOException
    {
        if(file.getName().endsWith(mime))
        {
            BufferedReader br = new BufferedReader(new FileReader(file));
            boolean IsGood = false;
            ArrayList<String> linestoadd = new ArrayList<String>();
            while(br.ready())
            {
                String line = br.readLine();
                if(!line.trim().equals(object.toString()))
                {
                    linestoadd.add(line.trim());
                }
                else
                {
                    IsGood = true;
                }
            }
            br.close();
            BufferedWriter fileclearer = new BufferedWriter(new FileWriter(file));
            fileclearer.write("");
            fileclearer.close();
            PrintWriter pw = new PrintWriter(new BufferedWriter(new FileWriter(file, true)));
            for(String linetoadd : linestoadd)  
            {
                pw.println(linetoadd);
            }
            pw.close();
            return IsGood;      

        }
        else
        {
            return false;
        }
    }

    public static ArrayList<String> getContents(File file) throws IOException
    {
        ArrayList<String> contents = new ArrayList<String>();
        if(file.getName().endsWith(mime))
        {
            BufferedReader br = new BufferedReader(new FileReader(file));
            while(br.ready())
            {
                contents.add(br.readLine());
            }
            br.close();
        }
        return contents;
    }
}

Answer 1

Reduce duplicated logic by using helper methods

Instead of repeating this condition:

if (file.getName().endsWith(mime)) {

Use a helper method:

private static boolean isValidFile(File file) {
    return file.getName().endsWith(mime);
}

The advantage is that this hides the implementation details of what is a valid file to you. Without this, all you methods need to be aware that the file name should end with mime, which is too much detail and duplicated logic. With the isValidFile helper, if you need to change something about the condition on the File object, you will be able to make the change in one place, inside isValidFile, without having to touch the rest of the program.

Bugs and improvements in containsObject

You don't exit the loop even after you found a match, iterating until the end of the file, which is a waste:

while (br.ready()) {
    String line = br.readLine();
    if (line.trim().equals(object.toString())) {
        IsGood = true;
    }
}

After you set IsGood = true, add a break;

IsGood is a bad name for 2 reasons:

• Variables should be camelCase
• It doesn't explain it self

foundMatch would make more sense.

Simplify and clean up removeObject

The "file clearer" is completely pointless. Instead of this:

BufferedWriter fileclearer = new BufferedWriter(new FileWriter(file));
fileclearer.write("");
fileclearer.close();
PrintWriter pw = new PrintWriter(new BufferedWriter(new FileWriter(file, true)));

You can write simply this for the same effect:

PrintWriter pw = new PrintWriter(new BufferedWriter(new FileWriter(file)));

Notice that I dropped the second true parameter from the the FileWriter when creating pw. This makes the writer truncate the file.

---

Instead of this:

ArrayList<String> linestoadd = new ArrayList<String>();

You should declare variables with interface types, in this case a List:

List<String> linestoadd = new ArrayList<String>();

And instead of linestoadd, I would name the variable content, or contentLines, but that may be a matter of taste.

---

Again, IsGood doesn't tell what it's about. If you rename to foundMatch, its purpose becomes much clearer.

Improve getContents

As I noted earlier about linestoadd in removeObject, you should declare variables, and also methods, using interface types. So instead of:

public static ArrayList<String> getContents(File file) throws IOException {
    ArrayList<String> contents = new ArrayList<String>();

This would be better:

public static List<String> getContents(File file) throws IOException {
    List<String> contents = new ArrayList<String>();

Inverting the if-else branches

All your methods have this pattern:

if (isValidFile(file)) {
    // ... (the main code)
} else {
    return false;
}

In cases, the main code is relatively long. As such, when you look at the else block, it's not visible what it is "the else of", I have to scroll up to see. In such situations, it would make the code more readable to invert the logic in the if-else: bring the much shorter block up, like this:

if (!isValidFile(file)) {
    return false;
} else {
    // ...
}

In fact, since in this case the block is a return statement, you can even eliminate the else block, reducing the nesting level, which is usually easier to read:

if (!isValidFile(file)) {
    return false;
}
// ...

Answer 2

I hate using other database programs

Forget this ASAP!, Database programmers are working hard to solve problems what are you don't understand yet. Reading the file each time is a performance hell, and not thread safe too. This is an text "database" so why accepts any kind of Objects? This database writes any object into the file, but could not convert it back to the original object.

Other hints:

• PrintWriter is AutoCloseable, so you should use Java7 try to close it.
• PrintWriter has many constructors, so new PrintWriter(new BufferedWriter(new FileWriter(file, true))) is not the easiest way to create one.
• You don't need to write else, because the in the if branch the code returns, so else is not necessary.
• Keep code DRY!
• This class has only static methods. java is an object oriented language, and You have to avoid "static classes" in most of the time. If You are one of your best friends, You learn OOP paradigm first.

Answer 3

I hate using other database programs

this is not a good starting point. Your implementation is very far from being comparable to any database program.

What you are doing:

• any Java object is converted to a (hopefully) single line of text
• you can add an object to a file (this is efficient)
• you can ask wether the file contains an object (linear search)
• you can remove an object (linear search + entire file rewriting)

weak points:

• slow search and removal of objects. You cannot hope to deal with more than a thousand of objects.

• if you deal with less than a thousand of objects it would be preferable to keep them all in memory and serialize/deserialize them all together to a file

• you cannot make searches based on objects properties. If you have a phonebook you must know the phone number of the people to find them.

Answer 4

1. You are writing Objects and reading Strings. There is no way for me to store a LolCat object in your dictionary (I need to organize my lolcats!) and then read a LolCat back from the database later (my lolcats are gone!). That doesn't even cover the fact that an object without an implementation of toString may have a different value from one instance of the VM to the next. Consider writing Serializable objects instead, as this will allow you to read in the objects that were written. (Note: You will need to have an ObjectOutputStream wrapped around your FileWriter.)

   This does put some additional pressure on consumers of your database code, but it's not much different than what you've currently got, which would force them to write some kind of parse(String s) method to get their LolCat back from the DB.

2. Your code assumes that every record will be a single line. What if I write LolCat like this:

   public String toString()
   {
       return this.filename + "\n"
            + this.caption + "\n"
            + "Lolz Magnitude: " + this.magnitude;
   }
   

   Your database will never find my glorious LolCats! You need some method of determining the location of a record other than simply separating them by newlines – or separating them by any string of characters, to tell the truth. No matter what separator you use, someone could potentially use that separator in the serialization of their object (whether you begin accepting Serializable objects or continue to use toString).

   One option might be to create an indexing file, which keeps track of the start and end location of records in your database (or start location and record length). If you're storing the start positions of your records, this also lets you improve your ability to find records, as you'll be able to use BufferedReader.skip to jump straight to the record you want. Of course, you still need to figure out how to define the index itself. Don't forget to keep the indexes up-to-date when records are added, removed, and changed!

Answer 5

The other answers already addressed the core issues and have good suggestions. I'd like to focus on smaller things that may cause your current implementation to misbehave.

• file.getName().endsWith(mime) will match both data.wtdb as oowtdb. If you'd like to match filename extensions only, consider using ".wtdb".

• Files are read and written without a defined encoding. This means the JVM will default to the platform encoding, which may differ between invocations, between operating systems, and between users. Even the definition of a line differs across platforms!

br.ready() returns true when the implementation can guarantee that the next read won't block:

True if the next read() is guaranteed not to block for input, false otherwise. Note that returning false does not guarantee that the next read will block.

That means that a stream may claim to be ready when there is no more data to be read (and hence will not block), and it may claim not to be ready when no extra data has buffered or the stream has encountered a brief hiccup.

If you want to fully exhaust a stream, call one of the read functions until it returns -1, or BufferedReader.readLine() until it returns null:

for ( String line = br.readLine(); line != null; line = br.readLine() ) {
    line = line.trim();
    // ...
}

String line;
while ( (line = br.readLine()) != null ) {
    line = line.trim();
    // ...
}

---

If you take this as an exercise in reinventing-the-wheel (which can be a learning if frustrating experience), keep the following in mind:

• Databases tend to be more randomly accessed than sequentially so. Take a look at mapping a file into memory for some OS support in random access.

• As mhmxs noted, the current implementation is not thread-safe. For (some) safety from out-of-process modification, file locking is a good start. For internal thread safety, you'll have to synchronize manually.

• Are you going to support transactions with commit and rollback? Record locking? Table locking? Indexing? Concurrency? Versioning? Don't get carried away! Be clear in what you will and, most importantly, what you will not support.