1
\$\begingroup\$

I'm learning Java recently, this is one of my first program written in this language.

I reimplemented a basic version of Fortune. This version prints to the standard output a randomly chosen line from a text file. You can specify the text file passing its name on the command line, otherwise the program reads the line from /usr/games/lib/fortune (but it will fail because the file probably doesn't exist).

You can compile the program with javac Fortune.java and run it with java Fortune fortune.txt.

Fortune.java

import java.nio.file.Paths;
import java.nio.file.Path;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;

import java.util.Random;
import java.util.ArrayList;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

/**
 * This program prints to the standard output a randomly line chosen from a text
 * file. You can specify the text file passing it as a command line argument,
 * otherwise it reads from '/usr/games/lib/fortune'.
 */
public class Fortune {
    private static BufferedReader inputFile;

    private static final String standardFilename = "/usr/games/lib/fortunes";

    public static void main(String[] args) {
        init(args);

        String randomAphorism = getRandomAphorism();

        System.out.println(randomAphorism);

        deinit();
    }

    /**
     * This method parses the command line arguments and initializes the field
     * 'inputFile'.
     * In case of errors it prints to stderr the error and quits.
     */
    private static void init(String[] args) {
        if (args.length > 1) {
            System.err.println("Error: it is required only one optional "
                               + "argument");
            System.exit(1);
        }

        String filename;

        if (args.length == 1)
            filename = args[0];
        else
            filename = standardFilename;

        try {
            Path source = Paths.get(filename);

            if (Files.notExists(source)) {
                System.err.println("File \"" + source + "\" doesn't exist or "
                                   + "can't be opened");
                System.exit(1);
            }

            inputFile = new BufferedReader(
                           new InputStreamReader(Files.newInputStream(source)));
        } catch (InvalidPathException e) {
            System.err.println(e.getMessage());
            System.exit(1);
        } catch (IOException e) {
            System.err.println(e.getMessage());
            System.exit(1);
        }
    }

    /**
     * It returns a randomly chosen aphorism from 'inputFile' field.
     * In case of errors it prints to stderr the error and quits.
     */
    private static String getRandomAphorism() {
        ArrayList<String> aphorisms = new ArrayList<>();

        try {
            String aphorism = inputFile.readLine();
            while (aphorism != null) {
                aphorisms.add(aphorism);
                aphorism = inputFile.readLine();
            }

        } catch (IOException e) {
            System.err.println(e.getMessage());
            System.exit(1);
        }

        Random random = new Random();
        int selectedAphorism = random.nextInt(aphorisms.size());

        return aphorisms.get(selectedAphorism);
    }

    /**
     * It closes 'inputFile' field.
     */
    private static void deinit() {
        try {
            inputFile.close();
        } catch (IOException e) {
            System.err.println(e.getMessage());
            System.exit(1);
        }
    }
}

Fortune.txt

Some men are discovered; others are found out.
Words must be weighed, not counted.
By failing to prepare, you are preparing to fail.
He who spends a storm beneath a tree, takes life with a grain of TNT.
You attempt things that you do not even plan because of your extreme stupidity.
Take care of the luxuries and the necessities will take care of themselves.
Words are the voice of the heart.
Your mind understands what you have been taught; your heart, what is true.
A king's castle is his home.
He who has a shady past knows that nice guys finish last.
The universe is laughing behind your back.
The best prophet of the future is the past.
It is a poor judge who cannot award a prize.
Even the boldest zebra fears the hungry lion.
Money will say more in one moment than the most eloquent lover can in years.
Money may buy friendship but money can not buy love.
Might as well be frank, monsieur. It would take a miracle to get you out of Casablanca.
Creditors have much better memories than debtors.
Many pages make a thick book.
Every purchase has its price.
Do not underestimate the power of the Force.
You will step on the night soil of many countries.
Mind your own business, Spock. I'm sick of your halfbreed interference.
He who invents adages for others to peruse takes along rowboat when going on cruise.
Of all forms of caution, caution in love is the most fatal.
If you suspect a man, don't employ him.
The Tree of Learning bears the noblest fruit, but noble fruit tastes bad.
Stop searching forever. Happiness is unattainable.
A man who fishes for marlin in ponds will put his money in Etruscan bonds.
A good memory does not equal pale ink.
How sharper than a hound's tooth it is to have a thankless serpent.
You dialed 5483
It's later than you think.
Mistakes are oft the stepping stones to failure.
It's not reality that's important, but how you perceive things.
Promptness is its own reward, if one lives by the clock instead of the sword.
Like winter snow on summer lawn, time past is time gone.
Far duller than a serpent's tooth it is to spend a quiet youth.
Let not the sands of time get in your lunch.
The attacker must vanquish; the defender need only survive.
Standing on head makes smile of frown, but rest of face also upside down.
Deprive a mirror of its silver and even the Czar won't see his face.
Man's horizons are bounded by his vision.
To criticize the incompetent is easy; it is more difficult to criticize the competent.
He who has imagination without learning has wings but no feet.
Men seldom show dimples to girls who have pimples.
Troglodytism does not necessarily imply a low cultural level.
You cannot kill time without injuring eternity.
As goatherd learns his trade by goat, so writer learns his trade by wrote.
One man tells a falsehood, a hundred repeat it as true.
Crazee Edeee, his prices are INSANE!!!
It is better to wear out than to rust out.
When the wind is great, bow before it; when the wind is heavy, yield to it.
The wise shepherd never trusts his flock to a smiling wolf.
It is the wise bird who builds his nest in a tree.
How you look depends on where you go.
A plucked goose doesn't lay golden eggs.
A man who turns green has eschewed protein.
Put not your trust in money, but put your money in trust.
Even a hawk is an eagle among crows.
Even the smallest candle burns brighter in the dark.
People who take cat naps don't usually sleep in a cat's cradle.
A truly wise man never plays leapfrog with a Unicorn.
Do not clog intellect's sluices with bits of knowledge of questionable uses.
Let him who takes the Plunge remember to return it by Tuesday.
Try to divide your time evenly to keep others happy.
You have mail.
His heart was yours from the first moment that you met.
Sin has many tools, but a lie is the handle which fits them all.
Let a fool hold his tongue and he will pass for a sage.
With clothes the new are best, with friends the old are best.
He is truly wise who gains wisdom from another's mishap.
Beware of a dark-haired man with a loud tie.
Today is the last day of your life so far.
Flee at once, all is discovered.
Man who falls in vat of molten optical glass makes spectacle of self.
Go directly to jail. Do not pass Go, do not collect $200.
For a good time, call 8367-3100.
Those who can, do; those who can't, simulate.
Those who can, do; those who can't, write. Those who can't write work for the Bell Labs Record.
God does not play dice.
This fortune is inoperative. Please try another.
Laugh, and the world ignores you. Crying doesn't help either.
No amount of genius can overcome a preoccupation with detail.
You will feel hungry again in another hour.
You now have Asian Flu.
God made the integers; all else is the work of Man.
Disk crisis, please clean up!
You auto buy now.
Many are called, few are chosen. Fewer still get to do the choosing.
Try the Moo Shu Pork. It is especially good today.
Many are cold, but few are frozen.
The early worm gets the bird.
He who hesitates is sometimes saved.
Time is nature's way of making sure that everything doesn't happen at once.
The future isn't what it used to be. (It never was.)
Can't open /usr/lib/fortunes.
If God had wanted you to go around nude, He would have given you bigger hands.
It is better to have loved and lost than just to have lost.
A journey of a thousand miles begins with a cash advance from Sam.
Disk crunch - please clean up.
Center meeting at 4pm in 2C-543
I will never lie to you.
Spock: We suffered 23 casualties in that attack, Captain.
Your computer account is overdrawn. Please reauthorize.
1 bulls, 3 cows
It's hard to get ivory in Africa, but in Alabama the Tuscaloosa.
Waste not, get your budget cut next year.
Old MacDonald had an agricultural real estate tax abatement.
Snow Day - stay home.
Save gas, don't eat beans.
All that glitters has a high refractive index.
Ignore previous fortune.
When in doubt, lead trump.
23. ... r-q1
unix soit qui mal y pense
Even a cabbage may look at a king.
Honi soit la vache qui rit.
No directory
Don't eat yellow snow.
One Bell System - it works.
One Bell System - it sometimes works.
* UNIX is a Trademark of Bell Laboratories.
chess tonight
External Security:
Peters hungry, time to eat lunch.
MOUNT TAPE U1439 ON B3, NO RING
A foolish consistency is the hobgoblin of little minds.
IOT trap -- core dumped
IOT trap -- mos dumped
/usr/news/gotcha
Rotten wood can not be carved - Confucius (Analects, Book 5, Ch. 9)
System going down at 1:45 this afternoon for disk crashing.
: is not an identifier
Quantity is no substitute for quality, but its the only one we've got.
Those who can do, those who can't, write.
The more things change, the more they'll never be the same again.
New crypt. See /usr/news/crypt.
You might have mail.
You can't go home again, unless you set $HOME
You are in a maze of twisty little passages, all alike.
\$\endgroup\$
0
\$\begingroup\$

Thanks for sharing your code.


Please read (and follow) the Java Naming Conventions

e.g.: in your code standardFilename is a constant and should be named with UPPERCASE_ONLY.

Also in java method names starting with get have a special meaning. They are expected to only return a member variable without doing any processing.

Also a method name should explicitly describe what a method does. So a better name for getRandomAphorism() could be loadAndSelectRandomAphorism().


You use System.exit() to shut down the JVM when you detect a failure condition. You should not do that. Use Javas Exception mechanism to handle that:

private static void createFileReader(String[] args) throws Exception {
    if (args.length > 1) {
        throw new Exception("Error: it is required only one optional "
                           + "argument");
    }

    String filename;
    if (args.length == 1)
        filename = args[0];
    else
        filename = standardFilename;
    Path source = Paths.get(filename);

    if (Files.notExists(source)) {
        System.err.println("File \"" + source + "\" doesn't exist or "
                           + "can't be opened");
        System.exit(1);
    }

    inputFile = new BufferedReader(
                   new InputStreamReader(Files.newInputStream(source)));
}

public static void main(String[] args) {
   try{
     createReader(args);
     //...
   }catch(Exception e){
     System.err.println(e.getMessage());
   }
}

You structured your code into several methods. This is a good thing in the first place. But IMHO you failed to keep responsibilities together.

e.g. You work with the input file in all three methods. A better approach might be to have the init() method only doing it, but then it should not verify the imput parameter. This should be done in another method.

If you have only one method actually interacting with the file system you can close the FileReader immediately in this method preferably using *try with resource:

private static void readFile(String fileName){ 
    Path source = Paths.get(filename);
    if (Files.notExists(source)) {
        throw new Exception("File \"" + source + "\" doesn't exist or "
                           + "can't be opened");
    }

    // Java closes this automatically
    try(inputFile = new BufferedReader(
                   new InputStreamReader(Files.newInputStream(source)))){ 

     String aphorism = inputFile.readLine();
        while (aphorism != null) {
            aphorisms.add(aphorism);
            aphorism = inputFile.readLine();
        }
     }
}
\$\endgroup\$
  • \$\begingroup\$ Thanks for the reply. I have a question: you said that I shouldn't use System.exit() when I detect a failure condition (for example args.length > 1; if the user run the command with more arguments, isn't better to show a custom error message instead of a uncaught exception? \$\endgroup\$ – user146184 Feb 12 '18 at 10:06
  • 1
    \$\begingroup\$ @ema-pe "isn't better to show a custom error message instead of a uncaught exception?" my suggestion was to have a try/catch block in main where you catch any exception and print its message... \$\endgroup\$ – Timothy Truckle Feb 12 '18 at 12:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy