10
\$\begingroup\$

I am trying to reverse the lines of a file by reading the file through an array.

Is there a more efficient and quicker way of achieving this?

import java.util.Scanner;
import java.io.*;

class ReverseNumbers {

    public static void main(String[] args) throws FileNotFoundException {

        File file = new File("numbers.txt");
        Scanner Scan = new Scanner(file);

        int numoflines = 0;
        while(Scan.hasNextLine()) {
            Scan.nextLine();
            numoflines++;
        }
        Scan.close();
        Scan = new Scanner(file);

        int buffer[] = new int[numoflines]; //size

        for(int i=0; i<buffer.length; i++) {
            buffer[i] = Scan.nextInt();
        }

        for(int i=buffer.length-1; i>=0; i--) {
            int reverse = buffer[i];
            System.out.println(reverse);
        }
    }
}
\$\endgroup\$
15
\$\begingroup\$

The posted code does the following:

  1. Read all lines, just for counting
  2. Create an array with the right size
  3. Read all lines again, this time for storing
  4. Print all lines in reverse order

It's wasteful to read a file completely just to find the number of lines, and then need to read the file again to store the content. It would be better to store the lines in a data structure that can grow its capacity as needed, to eliminate the need to know the number of lines in advance. That is, use any List implementation.

Possible bug

The description doesn't mention that there is precisely one integer per line in the input. It must be the case, otherwise this code may not work well:

for(int i=0; i<buffer.length; i++) {
    buffer[i] = Scan.nextInt();
}

That is, if there are multiple numbers per line, then you will not read the entire file, only the first \$N\$ numbers. Similarly, if there are empty lines, then there may not be enough numbers, and then the .nextInt call will throw a NoSuchElementException.

The thing is, since there program doesn't use any integer operation, only read and print, there's no point reading integers. Keep in mind that when you read integers, you also incur a parsing step. It would make more sense to read the lines as strings, it would be simpler, and it seems that's all you really need anyway.

Suggested implementation

Here's an alternative, more efficient solution, that also simplifies the reversing logic using a LinkedList, with a descendingIterator:

LinkedList<String> lines = new LinkedList<>();
while (scanner.hasNextLine()) {
    lines.add(scanner.nextLine());
}
scanner.close();

Iterator<String> iterator = lines.descendingIterator();
while (iterator.hasNext()) {
    System.out.println(iterator.next());
}
\$\endgroup\$
  • 3
    \$\begingroup\$ Consider using LinkedList.descendingIterator(). Jumping to a specific index in a LinkedList could be O(N). \$\endgroup\$ – 200_success Feb 19 '15 at 21:10
  • \$\begingroup\$ Ok maybe that's cleaner. I rewrote to use your suggestion. Thanks! \$\endgroup\$ – janos Feb 19 '15 at 21:25
4
\$\begingroup\$

class ReverseNumbers {

    public static void main(String[] args) throws IOException {

        URL url = ReverseNumbers.class.getResource("/numbers.txt");

        List<String> values = Files.readAllLines(Paths.get(url.getPath()));
        Collections.reverse(values);

        for(String s : values) {
            System.out.println(Integer.valueOf(s));
        }
    }
}

In terms of code, the above is, obviously, much more readable and basically uses the NIO classes for efficient management of file resources and Collections API for managing the list itself.

Also notice the getResource(String) can be a better way of loading resources as explained here.

There is also a nice discussion on features of using Collections API over arrays.

I suppose Scanner is good if you are expecting different types of input in your file. Considering the fact that the file is called "numbers.txt" then you can do everything in a single read and manage the resulting collection in whichever way you want, rather than having to scan the file.

\$\endgroup\$
  • \$\begingroup\$ what do you mean by would much rather use SLF4J? What does this mean? This doesn't make any sense. Also reversing the collection is unnecessarily expensive, you should just loop backwards. \$\endgroup\$ – Boris the Spider Feb 19 '15 at 21:13
  • \$\begingroup\$ @BoristheSpider What I meant was that Considering he wishes to print the collection reversed it would be easier to do it in one call using SLF4J: logger.info("{}", values) (which will call toString() on the collection) than having to iterate through the list of values. You can get more information on SLF4J here. \$\endgroup\$ – xelamitchell Feb 19 '15 at 21:17
  • 3
    \$\begingroup\$ I know what SLF4J is! It's a logging library, you know. For logging. It should not be used for formatted output. If the file needs to be output to the console in reverse, System.out is the correct approach. A logging library adds all sorts of other things, timestamps. Code locations. Etc. \$\endgroup\$ – Boris the Spider Feb 19 '15 at 21:19
  • \$\begingroup\$ Also - what makes you think the OP's file is on the classpath? \$\endgroup\$ – Boris the Spider Feb 20 '15 at 8:38
3
\$\begingroup\$

Taking all the points in Janos' answer and converting to Java 8 you have:

public static void main(String[] args) throws Exception {
    try (final Stream<String> lines = Files.lines(Paths.get("numbers.txt"))) {
        lines.collect(Collectors.toCollection(LinkedList::new))
                .descendingIterator()
                .forEachRemaining(System.out::println);
    }
}

You still need the intermediate collection, as you cannot reverse a Stream without consuming it all.

This solution does loose the the Scanner though, which is known to be very slow.

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

Assuming that the array isn't something you have to use, the natural solution to this kind of problem is to use a Stack. A stack is a data structure that is Last-In, First-Out.

The idea is that you read the file, one item(number, in your case) at a time, and 'push' each item on to the stack. Once you've read all of the items, you 'pop' them off the stack to display them.

Here's an example of how to do it with a stack:

Stack<Integer> numbers = new Stack<Integer>();
File file = new File("numbers.txt");
Scanner scanner = new Scanner(file);

while(scanner.hasNextInt())
{
    Integer next = scanner.nextInt();
    numbers.push(next);
}

while(!numbers.empty())
{
    Integer top = numbers.pop();
    System.out.print(reverse + " ");
}

A few notes:

  • The reason the stack is declared as Stack<Integer> instead of Stack<int> is because int is a primitive type, which means it doesn't inherit from Object. Integer does inherit from Object, so we use that instead. There's a bit of a rabbit hole there but I won't delve deeper. Other primitive types in Java(boolean, double, float, etc) have an analogous Object type(Boolean, Double, Float, etc).

  • Pop is kind of tricky. It's doing two things: it is returning the object on top of the stack, and it is removing that object from the stack. Since it removes an element from the stack, while(!numbers.empty()) will eventually be false and our loop will have run its course.

The downside to this approach is that you no longer have your numbers after the fact, since the pop functionality throws your numbers away. There are still ways to get around this, but that's something you have to keep in mind.

\$\endgroup\$

Your Answer

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

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