HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Simple text DataBase

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
databasetextsimple

Problem

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 linestoadd = new ArrayList();
while(br.ready())
{
String line = br.readLine();
if(!line.trim().equal

Solution

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 linestoadd = new ArrayList();


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

List linestoadd = new ArrayList();


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 getContents(File file) throws IOException {
    ArrayList contents = new ArrayList();


This would be better:

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


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;
}
// ...

Code Snippets

if (file.getName().endsWith(mime)) {
private static boolean isValidFile(File file) {
    return file.getName().endsWith(mime);
}
while (br.ready()) {
    String line = br.readLine();
    if (line.trim().equals(object.toString())) {
        IsGood = true;
    }
}
BufferedWriter fileclearer = new BufferedWriter(new FileWriter(file));
fileclearer.write("");
fileclearer.close();
PrintWriter pw = new PrintWriter(new BufferedWriter(new FileWriter(file, true)));
PrintWriter pw = new PrintWriter(new BufferedWriter(new FileWriter(file)));

Context

StackExchange Code Review Q#63318, answer score: 11

Revisions (0)

No revisions yet.