patternjavaModerate
Simple text DataBase
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
```
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:
Use a helper method:
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
Bugs and improvements in
You don't exit the loop even after you found a match, iterating until the end of the file, which is a waste:
After you set
Simplify and clean up
The "file clearer" is completely pointless. Instead of this:
You can write simply this for the same effect:
Notice that I dropped the second
Instead of this:
You should declare variables with interface types, in this case a
And instead of
Again,
Improve
As I noted earlier about
This would be better:
Inverting the if-else branches
All your methods have this pattern:
In cases, the main code is relatively long. As such, when you look at the
In fact, since in this case the block is a
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
containsObjectYou 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
removeObjectThe "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
getContentsAs 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.