Hacker News new | ask | show | jobs
by ebola1717 3431 days ago
In practice, your first example would be more likely to look like:

    public List<Integer> add2(File file) {
        List<Integers> nums = file.getNumbers();
        if(nums != null) {
            List<Integer> resultList = List<>();
            for ( i : nums) {
                resultList.append(i + 2);
            }
            return resultList;
        } else {
            return null;
        }
    }
Compare that to:

    public Optional<List<Integer>> add2(File file) {
        Optional<List<Integers>> numsOpt = file.getNumbers();
        return numsOpt.map((nums) =>
            nums.map((i) => i + 2);
        );
    }
I find it hard to argue that the latter is worse.
2 comments

    public List<Integer> add2(File file) {
      List<Integer> resultList = new ArrayList<>();
      for (int i : ListUtils.emptyIfNull(file.getNumbers())) {
        resultList.append(i + 2);
      }
      return resultList;
    }
And now you need to use a non-language builtin util everywhere. And this one only works when working with arrays or array like objects...

It's amazing how far people will go to defend a bad practice, than even its inventor called a 'billion dollar mistake'.

This returns an empty list when file fails to read, which is not the expected behavior.

That's a case that comes up a lot! DB read failed vs DB read 0 items for example.

    public List<Integer> add2(File file) {
        List<Integers> nums = file.getNumbers(); // Show where the data comes from

        if(nums == null) { // Show the invariant provided for the rest of the code 
            return Collections.<Integer>emptyList(); // Show the default return value if there has been an error
        }

        List<Integer> resultList = List<>(); // Apply your logic
        for ( i : nums) {
            resultList.append(i + 2);
        }
        return resultList; // Return your list
    }
Then you document that behavior in a javadoc. Throw in @Nullable where applicable and move on. Or you can do this

    public List<Integer> add2(File file) {
        // Show data source
        List<Integers> nums = file.getNumbers();

        // Show case where data is supplimented
        if(nums == null) 
            nums = Collections.<Integer>emptyList(); // Show supplimnetal

        // Show transformation
        List<Integer> resultList = List<>();
        for ( i : nums) {
            resultList.append(i + 2);
        }
        return resultList;
    }
This is important to unify because nums won't always be null. We also won't always be supplimenting with an empty set. I don't see what value is recived from compressing that all into a single line. I don't see that as being more readable. I do see this very simple, step by step, explination of what's happening as being dead simple that no one can misunderstand.