1
\$\begingroup\$

Often I write notes about a topic into a simple text file when I learn something. Sometimes i work with these text file on other computers with a different screen resolution. So sometimes the lines of the file are to long for printing them in one displaying line.

I have written a program that solves that problem. I want to know what you think about my design.

Usage:

java Sequence inputFile outputFile charsPerLine

Code:

import java.io.File;
import java.io.FileReader;
import java.io.PrintWriter;
import java.io.IOException;

public class Sequence {
    private String inputFileName;
    private String outputFileName;
    private File inputFile;
    private File outputFile;
    private String[] symbols;
    private int symbolsUsed;
    private int lineLength;

    public Sequence (String inputFileName, String outputFileName, int lineLength, boolean doWork) throws Exception {
        inputFile = new File(inputFileName);
        outputFile = new File(outputFileName);
        this.lineLength = lineLength;
        symbolsUsed = 0;
        if(doWork) {
            analyzeSymbols();
            writeToFile();
        }
    }

    public void analyzeSymbols() throws Exception {
        symbols = new String[10000];
        for(int i = 0; i < 10000; i++) {
            symbols[i] = "";
        }
        FileReader fr = new FileReader(inputFile);
        int n = 0;
        char c = 0;
        while (n != -1) {
            n = fr.read();
            c = (char)n;
            if (c == '\n') {
                symbolsUsed++;
                symbols[symbolsUsed] = "force newline";
                symbolsUsed++;
            } else if (c == ' '){
                symbolsUsed++;
            } else if (c == '\t') {
                symbols[symbolsUsed] += c;
                symbolsUsed++;
            } else {
                symbols[symbolsUsed] += c;
            }
        }
        symbolsUsed++;
    }

    public void writeToFile() throws Exception {
        String text = "";
        int actualLineLength = 0;
        boolean lineWithT = false; // \t
        String tString = "";
        int numberOfT = 0;
        int i = 0;
        while(i < symbolsUsed) {
            if(actualLineLength + symbols[i].length() + 1 <= lineLength && !symbols[i].equals("\t") && !symbols[i].equals("force newline")) {
                text += symbols[i] + " ";
                actualLineLength += symbols[i].length() + 1;
                i++;
            } else if (symbols[i].equals("\t")) {
                lineWithT = true;
                tString = "\t";
                actualLineLength += 4; // i hope that the used editor marks a \t with 4 whitespace symbols, should be standard
                i++;
                numberOfT++;
                while(symbols[i].equals("\t")) {
                    tString += "\t";
                    actualLineLength += 4;
                    i++;
                    numberOfT++;
                }
                text += tString;
            } else if(symbols[i].equals("force newline")) {
                text += "\n";
                actualLineLength = 0;
                i++;
                if(tString != "") {
                    tString = "";
                    numberOfT = 0;
                    lineWithT = false;
                }
            } else if(actualLineLength + symbols[i].length() + 1 > lineLength) {
                text += "\n";
                actualLineLength = 0;
                if(lineWithT) {
                    text += tString;
                    actualLineLength += numberOfT * 4;
                }
            }
        }
        PrintWriter pr = new PrintWriter(outputFile);
        pr.println(text);
        pr.close();
    }

    public static void main(String[] args) throws Exception {
        Sequence s = new Sequence(args[0], args[1], Integer.parseInt(args[2]), true);
    }
}

Example

Input file

Dieses Programm liest einen Text ein und schreibt diesen in eine neue Datei, wobei die Zeilen eine gewisse Anzahl an Zeichen nicht übersteigt.
Es müssen einzelne Symbole unterschieden werden. Die Unterscheidung erfolgt durch das Erkennen von Leerzeichen und Zeilenumbrüchen.

Programmschritte:
    Erkenne Symbole
    Schreibe Symbole in neue Datei, überschreite Dateilänge nicht
    Hier steht noch unnötiger Text zu Demonstrationszwecken:
        Habt euch alle lieb
        tut euren Nachbarn gutes

1. Thema
    - Erläuterung lorem Ipsum dolorenti für 400 Euro
    - Spaß ist was ganz tolles und macht Freude
    - Do you know de way? We must save the queen. Everyone clap for the queen
2. Blumentopf

Output file with 30 characters per line

Dieses Programm liest einen 
Text ein und schreibt diesen 
in eine neue Datei, wobei die 
Zeilen eine gewisse Anzahl an 
Zeichen nicht übersteigt. 
Es müssen einzelne Symbole 
unterschieden werden. Die 
Unterscheidung erfolgt durch 
das Erkennen von Leerzeichen 
und Zeilenumbrüchen. 

Programmschritte: 
    Erkenne Symbole 
    Schreibe Symbole in neue 
    Datei, überschreite 
    Dateilänge nicht 
    Hier steht noch unnötiger 
    Text zu 
    Demonstrationszwecken: 
        Habt euch alle lieb 
        tut euren Nachbarn 
        gutes 

1. Thema 
    - Erläuterung lorem Ipsum 
    dolorenti für 400 Euro 
    - Spaß ist was ganz 
    tolles und macht Freude 
    - Do you know de way? We 
    must save the queen. 
    Everyone clap for the 
    queen 
2. Blumentopf
\$\endgroup\$

3 Answers 3

1
\$\begingroup\$

Here are my comments:

  1. instance variables inputFileName and outputFileName never get initialized and are never used.

  2. input file is never closed.

  3. some variables are declared out of the scope they should be. For instance char c in analyzeSymbols() is declared outside the loop but is initialized and used only inside it.

  4. It will be more efficient to read the entire input file to memory. Java 7 has methods that do just that Files.readAllBytes() and Files.readAllLines() that returns List of Strings according to the input lines so you already know where new line was in input

  5. There is no exception handling. to have main() throw the exception is not good practice for exception handling.

\$\endgroup\$
1
\$\begingroup\$

The problem you tried to tackle is very non-trivial. You may find this enlightening.

Therefore I was sure your code may not possibly work correctly and I made few quick tests. And indeed, too many bugs.

  • A bug? A word longer than charsPerLine argument cause the program to hang.

  • Another bug?

    I made a file baz (4 spaces as separators):

    aaaa    bbbb    cccc    dddd    eeee
    

    and running java Sequence baz bar 30 produced

    aaaa    bbbb    cccc    
    dddd    eeee 
    

    Notice a funny character at the end; the od -x1 bar reveals

    0000000    20  20  20  20  61  61  61  61  20  20  20  20  62  62  62  62
    0000020    20  20  20  20  63  63  63  63  20  20  20  20  0a  64  64  64
    0000040    64  20  20  20  20  65  65  65  65  20  0a  ef  bf  bf  20  0a
    

    Where did those ef bf bf 20 came from?

    Also notice that the line was broken after 24 symbols. It shall accommodate six more (dddd).

  • Yet another bug?

    I modify the baz file to have tabs instead of spaces. This is the od of the result:

    0000000    09  61  61  61  61  09  20  62  62  62  62  09  20  63  63  63
    0000020    63  09  20  64  64  64  64  09  20  0a  09  65  65  65  65  20
    0000040    0a  ef  bf  bf  20  0a
    

    Where did those spaces (hex 20) after each tab (hex 09) came from?

At this point I lost interest in QAing your code.

  • I don't see a reason to rewrite '\n' to "force newline".

  • doWork doesn't seem to have a purpose.

\$\endgroup\$
1
  • \$\begingroup\$ Good gracious. I did not thought there were so many mistakes in the code. I had previously tested the code in detail on my own text files and found no inconvenience. The problem is probably that I have designed the program only for my writing style, and did not consider general cases. If the code is so bent, it would probably be better to take it down, since the site is actually meant only for functioning code. \$\endgroup\$ Commented Jan 22, 2018 at 8:37
0
\$\begingroup\$

never do work in a constructor

You call your business logic from the constructor. This means you cannot instantiate the class without this logic is executed immediately. In consequence you cannot create subclasses to change the behavior since the logic runs before the subclass is completely initialized.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.