Optional

[Masynchin]

Hello, @yegor256!

Why don't this project use Optional, but rather implements its own Opt? I think that Optional can safely replace Opt and also bring map method, which can be used to get rid of if opt.has() { /* do something with opt.get() */ } else { return Opt.Empty() }.

For example, this method

takes/src/main/java/org/takes/facets/auth/PsBasic.java

Lines 238 to 261 in 91d020c

	@Override
	public Opt<Identity> enter(final String user, final String pwd) {
	final Opt<String> urn = this.urn(user, pwd);
	final Opt<Identity> identity;
	if (urn.has()) {
	try {
	identity = new Opt.Single<>(
	new Identity.Simple(
	URLDecoder.decode(
	urn.get(), PsBasic.Default.ENCODING
	)
	)
	);
	} catch (final UnsupportedEncodingException ex) {
	throw new IllegalStateException(
	String.format("Failed to decode URN '%s'", urn.get()),
	ex
	);
	}
	} else {
	identity = new Opt.Empty<>();
	}
	return identity;
	}

Can be rewrite like this:

@Override
public Opt<Identity> enter(final String user, final String pwd) {
    return this.urn(user, pwd).map(urn -> {
        try {
            return new Identity.Simple(
                URLDecoder.decode(
                    urn, PsBasic.Default.ENCODING
                )
            );
        } catch (final UnsupportedEncodingException ex) {
            throw new IllegalStateException(
                String.format("Failed to decode URN '%s'", urn),
                ex
            );
        }
    });
}

It uses two less variables and one less if-else statement. I think this code is more readable and more maintainable.

[yegor256]

@Masynchin indeed, you are right. I can't remember why Opt was introduced. Maybe because we developed Takes for Java 6, where there was no Option. If you are interested, please, submit a pull request and let's get rid of Opt.

[Masynchin]

@yegor256 I am planning this refactor:

1. Introduce OptionalEnvelope.

It extends both Opt and Optional, so we can replace all Opt occurrences with OptionalEnvelope. Pseudocode:

class OptionalEnvelope extends Opt, Optional {
    private final Optional origin;

    get() { origin.get() }
    has() { origin.isPresent() }

    // ...all Optional methods delegated to `origin`
}

2. Use Optional-part functionality of OptionalEnvelope.

Since it is both Opt and Optional, we can start using Optional methods such as map, filter and others not breaking the old code that depends on Opt.

3. Get rid of has.

We can both replace it with isPresent or use map + orElse/orElseGet methods combinations.

4. Replace OptionalEnvelope with Optional.

After deleting has method, OptionalEnvelope has no difference with Optional, so we can replace it.

What do you think?

[yegor256]

@Masynchin you are planning to submit four pull requests?

[Masynchin]

@Masynchin you are planning to submit four pull requests?

@yegor256 No, just one PR - "Replace Opt with Optional". This is just a step-by-step plan. Does this plan have any issues? If it seems OK, I can start implementing it.

[Masynchin]

It turned out I can't just create OptionalEnvelope as it extends Optional implements Opt. This refactoring will take much longer.