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

Very basic Java file reader/writer

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

Problem

I started learning the Java language about 10 days ago, reading Introduction to Programming Using Java by David J. Eck. I've made it halfway through chapter six in about 9 days. I realized I was going way too fast to learn efficiently, and so I'm going back to reread the focal points of the chapters and finish the exercises in more detail.

I've written out a program that allows the user to choose to read or write to a file, whether it be existent or new. They also may choose to write to a file specified by a name, or chosen from the file tree via window (I did not code that part; I believe it is in the TextIO class). The reading class allows the user to read from the last file, or one specified from the file tree via window (see above).

Question: What are some ways to reformat the code for a more proper semantic approach, or for better flow of program? I'm concerned with how often Eclipse complained about referring to a static in a non-static field, and so (nearly?) everything had to be declared public static.
Also, what might be some good ways to extend a program like this - assuming my minimal experience with the language - and also provide a good learning curve/challenge?

Link to GitHub repository

```
public class Filer {

final static int read = 1;
final static int write = 2;

public static void main(String[] args) {

pl("");
pl("Welcome, and thank you for using Text File");
pl("Manager, Version 0.0 . ");
pl("");
pl("");
pl("This software was created by Phil Carpenter at");
pl("Tempest Design Studios © 2015");
pl("This program allows you to read and write to");
pl("files using a few different methods.");
pl("");
pl("");
pl("You may either choose to either write to a file, or");
pl("read from an existing file.");
pl("You may als

Solution

Don't repeat yourself

You do this kind of thing a lot:

pl("********************************************************");
pl("Welcome, and thank you for using Text File");
pl("Manager, Version 0.0 . ");
pl("");


It would be better to create a helper function for that:

private static final printSection(String ... lines) {
    pl("********************************************************");
    for (String line : lines) {
        pl(line);
    }
    pl("");
}


Using this helper, you could rewrite the example at the top simpler, safer, with less duplication as:

printSection(
    "Welcome, and thank you for using Text File",
    "Manager, Version 0.0 . "
);


Sometimes you print the many in other contexts too.
So to make that piece more reusable, you should put that in another helper,
for example:

private static final printHorizontalRule() {
    pl("********************************************************");
}


Constants

These look like intended as constants:

final static int read = 1;
final static int write = 2;


The convention for naming constants is SHOUT_CASE.
And the ordering of the final static modifiers should be static final as per the Java Language Specification.
Lastly, these constants are internal details of your implementation,
no need to be visible outside, so they should be private:

private static final int READ = 1;
private static final int WRITE = 2;


Code organization

Filer has a lot of logic in main.
That's not ideal.
The job of "main" is to do only essential setup required for the program to start doing something. The real logic of the program should be in other methods, with descriptive names, outlining their purpose / responsibility.

Naming

The convention for method names is camelCase,
so, for example, these names should change in FileRead:

  • Reader -> reader



  • ReadUser -> readUser



  • ReadLast -> readLast



Even more importantly, class names are typically nouns and method names are typically verbs, expressing actions taken on objects.
It's not intuitive what a FileRead object represents,
and what is the meaning of FileRead.reader.

Formatting

The indenting here is rather chaotic:

while(choice != read && choice != write){
               pl("********************************************************");
        p("Please choose a valid option [1 or 2]:    ");
        choice = TextIO.getlnInt();
    } // end while(choice) //


And you should place a space before ( and after ):

while (choice != read && choice != write) {
        pl("********************************************************");
        p("Please choose a valid option [1 or 2]:    ");
        choice = TextIO.getlnInt();
    } // end while(choice) //


Avoid System.exit

There's no need for this statement at the end of the main method:

System.exit(0);


The program will automatically exit after leaving the main method.

In general, System.exit is often misused by beginners,
try to avoid it when possible.
Let the program exit naturally.

Code Snippets

pl("********************************************************");
pl("Welcome, and thank you for using Text File");
pl("Manager, Version 0.0 . ");
pl("");
private static final printSection(String ... lines) {
    pl("********************************************************");
    for (String line : lines) {
        pl(line);
    }
    pl("");
}
printSection(
    "Welcome, and thank you for using Text File",
    "Manager, Version 0.0 . "
);
private static final printHorizontalRule() {
    pl("********************************************************");
}
final static int read = 1;
final static int write = 2;

Context

StackExchange Code Review Q#111457, answer score: 4

Revisions (0)

No revisions yet.