Ocena kodu - wczytywanie z pliku

Czesc. Napisałem prostą klaskę konfiguracyjną, która ma wczytywać adres url z pliku na classpathie i zwracac go w postaci stringa. Plik nie zawiera nic innego poza urlem.
Próbuje się wdrożyć w clean code, 'nowości' z javy 8 i biblioteke vavr. Mógłby ktoś podpowiedzieć czy tak się powinno korzystać z tych dobrodziejstw, a jeśli nie to co poprawić ? mi się wydaje, że popłynąłem z tymi Optionalami ale boje się, że nie tylko to :p

@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class UrlProvider {

    private static Logger logger = LoggerFactory.getLogger(UrlProvider.class);

    public static UrlProvider getInstance(){
        return new UrlProvider();
    }
    
    public String readUrl(String source){
        return fileLocation(source)
                .flatMap(this::readFile)
                .orElseThrow(()-> new IncorrectResourceException(String.format("Unable to read url from file %s", source)));
    }

    private Optional<Path> fileLocation(String source) {
        URL url = extractUrlByFileName(source);
        return Try.of(()-> readPathFromUrl(url))
                .map(Optional::ofNullable)
                .onFailure(e-> logger.error("Unable to read path from {}", url, e))
                .getOrElse(Optional.empty());
    }

    private URL extractUrlByFileName(String file) {
        return Optional.ofNullable(getClass().getClassLoader().getResource(file))
                .orElseThrow(()-> new UrlExtractionException(String.format("Could not build the resource url of %s", file)));
    }

    private Path readPathFromUrl(URL resource) throws URISyntaxException {
        return Paths.get(resource.toURI());
    }

    private Optional<String> readFile(Path path) {
       return Try.of(() -> streamFrom(path))
                .map(chars -> chars.collect(Collectors.joining("\n")))
                .filter(result -> !result.isBlank())
                .map(Optional::ofNullable)
                .onFailure(e-> logger.error("Unable to read the content of the file or the content is empty", e))
                .getOrElse(Optional.empty());
    }

    private Stream<String> streamFrom(Path path) throws IOException {
        return Files.lines(path);
    }
}

Jak już używasz Try, to raczej rzucanie wyjątku nie jest ok. Podobnie z Optionalem, chyba nic nie wnosi. Dlaczego metoda publiczna nie zwraca po prostu Try? Wtedy klient decyduje jak chce obsłużyć brak błąd. IMHO byłaby to bardzo intuicyjna sygnatura.

Jak wyżej.
W Twoim przypadku metody powinny raczej zwracać Try, bo Optional gubi informację co sie stało. Nie ma bo nie, a czasem jednak warto wiedzieć czemu.
Przede wszystkim Try powinno być zwracane przez streamFrom, wtedy kod będzie też prostszy.

W ogólności jako rezultat metody, w której coś może się nie udać, dobry jest Either<Bład, Rezultat>, gdzie Błąd to jakaś klasa opisująca problem enum lub cała hierarchia klas (dobrze sprawdzają się sealed class (java 15)). Błąd nie musi być Exceptionem (w sensnie nie musi dziedziczyć po Exception) - i zwykle dobrze, żeby nie był(!!!).
W bardzo szczególnej sytuacji, kiedy każdy błąd jest diagnozowany przez istniejące API, rzucające wyjątkami może się okazać, że i tak zwrócisz coś w stylu Either<Excepton, Rezultat>. Wtedy używasz Try bo, Try<A> == Either<Exception, A>.
Jak już używasz Try to nie rzucasz wyjątkami przeważnie. Jeśli metoda przyjmuje jako parametr String source i efektywnie powodem błędu jest to, że coś jest nie tak z tym source to nie zwracasz wyjątku tylko Try, (ewentualnie Either).
Wyjątek Exception rzuczasz tylko jeśli problemem jest coś na co wywołujący metodę nie ma wpływu, sytuacja wymknęła sie spod kontroli -u Ciebie nie widać takiego przypadku.

Optionala używasz, kiedy nie chcesz mówić czemu coś się nie udało, - nie ma bo nie - nie interesuj się. Przy okazji: zamiast zrypanego Optional z java 8 możesz użyć Option z vavr.

Dzięki wielkie za uwagi. Zrobiłem mały refactoring pod kątem tego co napisał Charles_Ray. Post jarekr000000 też mega cenny ale potrzebuje jeszcze chwilę na jego analizę :p
Na ten moment wygląda to tak :

@NoArgsConstructor(access = AccessLevel.PRIVATE)
public class UrlProvider {

    private static Logger logger = LoggerFactory.getLogger(UrlProvider.class);

    public static UrlProvider getInstance(){
        return new UrlProvider();
    }
    
    public Try<String> readUrl(String source){
       return fileLocation(source)
                .flatMap(this::readFile);
    }

    private Try<Path> fileLocation(String source) {
       return extractUrlByFileName(source)
                .flatMap(url -> Try.of(()-> readPathFromUrl(url))
                                .onFailure(e-> logger.error("Unable to read path", e)));
    }

    private Try<URL> extractUrlByFileName(String file) {
        return Try.ofSupplier(() -> getClass().getClassLoader().getResource(file))
                .filter(Objects::nonNull)
                .onFailure(e-> logger.error("File could not be found", e));
    }

    private Path readPathFromUrl(URL resource) throws URISyntaxException {
        return Paths.get(resource.toURI());
    }

    private Try<String> readFile(Path path) {
       return Try.of(() -> streamFrom(path))
                .map(chars -> chars.collect(Collectors.joining("\n")))
                .filter(result -> !result.isBlank())
                .onFailure(e-> logger.error("Unable to read the content of the file or the content is empty", e));
    }

    private Stream<String> streamFrom(Path path) throws IOException {
        return Files.lines(path);
    }
}

Lepiej to wygląda, o ile działa - dopisz testy.

Co daje to getInstance()?

Przy okazji to @NoArgsConstructor(access = AccessLevel.PRIVATE) też mocne.
Narypać Lomboka i całkiem nieżły kawałek adnotacji po to żeby nie napisać normalnego prywatnego konstruktora. Ciekawe.