Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refine AddressParser interface #3021

Open
msgilligan opened this issue Apr 13, 2023 · 8 comments
Open

Refine AddressParser interface #3021

msgilligan opened this issue Apr 13, 2023 · 8 comments
Labels
Developer discussion discussion between developers about a specific topic
Milestone

Comments

@msgilligan
Copy link
Member

msgilligan commented Apr 13, 2023

The AddressParser interface should probably change from:

public interface AddressParser {
    Address parseAddressAnyNetwork(String addressString) throws AddressFormatException;
    Address parseAddress(String addressString, Network network) throws AddressFormatException, AddressFormatException.WrongNetwork;

    @FunctionalInterface
    interface Strict {
        Address parseAddress(String addressString) throws AddressFormatException;
    }
}

to something more like:

@FunctionalInterface
public interface AddressParser {
    Address parseAddress(String addressString) throws AddressFormatException;

    interface NetworkAddressParser extends AddressParser {
        Address parseAddress(String addressString) throws AddressFormatException;
        Address parseAddressForNetwork(String addressString, Network network) throws AddressFormatException, AddressFormatException.WrongNetwork;
    }
}

This addresses the following issue with the current implementation:

  1. There are many use-cases where the @FunctionalInterface is useful and in this context it could be "strict" or "non-strict" (i.e. may or may not check the Network.
  2. "AnyNetwork" is a poor naming choice. It's "all networks known by the configuration of this parser"
  3. A specialized exception should be thrown for the case of the network not matching the provided value (this means the @FunctionalInterface can be used in either way)

Make sure to scroll to the right to see the AddressFormatException.WrongNetwork being thrown. We may also want to refine the class hierarchy of AddressFormatException slightly to so that WrongNetwork is a separate case (not a subclass of) InvalidPrefix (aka unknown network) and perhaps even one new case should be added.

@msgilligan msgilligan added this to the 0.17 milestone Apr 13, 2023
@schildbach schildbach added the Developer discussion discussion between developers about a specific topic label Apr 13, 2023
@schildbach
Copy link
Member

I still don't understand the implications of functional interfaces and these specific changes. But if you think it's an improvement why not give it a try?

@msgilligan
Copy link
Member Author

PR #3103 Uses a slightly different implementation than suggested above to address issues (1) and (2) above, but does not address issue (3).

@msgilligan
Copy link
Member Author

PR #3101 was closed in favor of PR #3129, which has been merged.

@schildbach
Copy link
Member

Is this issue still open for (3) above, or can it be closed because it has mostly been resolved?

@schildbach
Copy link
Member

If not closed, I tend to defer this to 0.18 too.

@msgilligan
Copy link
Member Author

Let's leave it open for at least another few days and I'll probably defer it to 0.18, but also open a new smaller issue for 0.17 and maybe create a PR to match. I also want to make a PR for validating already parsed addresses against networks as we recently discussed.

@schildbach
Copy link
Member

My understanding is you created the "smaller PR" as #3384 and it was merged. So can we defer this to 0.18?

@schildbach
Copy link
Member

I'm removing this as a blocker for 0.17, so that we can go ahead and branch off for beta/rc/release.

@schildbach schildbach modified the milestones: 0.17, 0.18 May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer discussion discussion between developers about a specific topic
Projects
None yet
Development

No branches or pull requests

2 participants