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

Converter<A, B>, which combines a function and its sort-of-inverse #177

Closed
gissuebot opened this issue Oct 31, 2014 · 30 comments
Closed

Converter<A, B>, which combines a function and its sort-of-inverse #177

gissuebot opened this issue Oct 31, 2014 · 30 comments
Assignees
Milestone

Comments

@gissuebot
Copy link

Original issue created by ejwinter on 2009-05-29 at 06:48 PM


In several projects I have added the following interface. I think it would
go very well in Google Collections:

public interface InvertableFunction<T,K> extends Function<T,K> {
    InvertableFunction<K,T> reverse();
}

With an example implementation of:

public class BasicEncryptor implements InvertableFunction<String,String>{
    private final BasicTextEncryptor encryptor;

public BasicEncryptor(String encryptionKey) {
    this.encryptor = new BasicTextEncryptor();
    encryptor.setPassword(encryptionKey);
}

@Override
public String apply(String rawString) {
    return encryptor.encrypt(rawString);
}

@Override
public InvertableFunction<String, String> invert() {
    return new InvertableFunction<String, String>() {

        @Override
        public InvertableFunction<String, String> reverse() {
            return BasicEncryptor.this;
        }

        @Override
        public String apply(String from) {
            return encryptor.decrypt(from);
        }
    };
}

}

@gissuebot
Copy link
Author

Original comment posted by kevinb9n on 2009-05-29 at 06:54 PM


I can see this happening eventually. It's a little problematic because so many
functions that have an inverse() function are not actually invertible in the
mathmetical sense (for example, you can convert "1.00" to a double and get 1.0, then
back to a String and get 1.0), and as a result, the kinds of things you expect to be
able to do with an invertible function (like Sets.transform(Set)) aren't really ironclad.

I think we can work out how to warn about the minor risks though.

@gissuebot
Copy link
Author

Original comment posted by kevinb9n on 2009-09-17 at 05:33 PM


I just realized that we have actually implemented this already internally, but we
have named it "Converter."

How would users feel if our InvertibleFunction class were named Converter instead?

Also, it's an abstract class instead of an interface so that it can have useful
methods on it like convertAll() (aka transform()). That's inconsistent with our
other types, but it is quite nice...


Status: Accepted
Labels: post-1.0

@gissuebot
Copy link
Author

Original comment posted by kevinb9n on 2009-09-17 at 05:34 PM


(No comment entered for this change.)

@gissuebot
Copy link
Author

Original comment posted by kevinb9n on 2009-09-17 at 05:45 PM


(No comment entered for this change.)


Labels: -post-1.0, Milestone-Post1.0

@gissuebot
Copy link
Author

Original comment posted by kevinb9n on 2009-09-17 at 05:57 PM


(No comment entered for this change.)


Labels: Type-Enhancement

@gissuebot
Copy link
Author

Original comment posted by ejwinter on 2009-09-17 at 06:16 PM


Could Converter<T,K> be made to implement InvertibleFunction<T,K>. Then you can
implement an interface as an option. The naming would be fine.
Thanks,
Eric

@gissuebot
Copy link
Author

Original comment posted by stephen.kestle on 2009-12-03 at 01:51 AM


Definitely make Converter implements InvertibleFunction.

IdentityFunction is one implementation :).

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2010-07-30 at 03:53 AM


(No comment entered for this change.)


Labels: -Milestone-Post1.0

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2011-01-27 at 01:33 PM


(No comment entered for this change.)


Labels: Milestone-Release09

@gissuebot
Copy link
Author

Original comment posted by jim.andreou on 2011-03-16 at 10:46 PM


Just a comment from my recent experience implementing something like a Sets#transform: the tricky part is to specify the bijection contract. I mean the exact semantics of the methods, not just the signatures.

For example:
public interface InvertibleFunction<A,B> extends Function<A,B> {
    InvertableFunction<B,A> reverse();
}

This, without any other specification, implies a bijection A <--> B, i.e. from every A to every B (oh, yes, A and B (infinite) sets should have the same cardinality too). This specification makes it the easiest to implement Sets#transform, but also the hardest to implement InvertibleFunction correctly.

At least from the perspective of Sets#transform, what is really needed is something far less, and far easier to implement: a bijection not from all A to all B, but one that needs only to be defined for those A's in a specific Set<A>. The image of that would be a Set<B>, and similarly, the inverse function would only be required to map only the B's in that Set<B> back to A's of Set<A>.

InvertibleFunction as given above doesn't make it easy to restrict ourselves to a smaller domain than the type parameter itself (Set<A> is just a subset of A).

Aside: scala functions have an "isDefinedAt(x: A)" method, which is precisely what I'm talking about. They don't define functions over all A, but only a subset of it (a predicate on elements of A, such as isDefinedAt, defines a set of A just as Set<A> does, modulo not providing an iterator).

But we have no isDefinedAt method, and we can't define one, and even if we could, interfaces with two methods are going to become much more costly than those with a single method when closures arrive, so I think we are pretty much stuck with the clumsier approach. The good news is that the clumsier approach is workable, but the specification of a potential Sets#transform would be forced to explain this subtlety, that not a complete bijection is required, but only one defined for the given Set<A> and its image Set<B>, for other inputs it would be free to return whatever.

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2011-03-18 at 10:22 PM


I cross-posted the above in #219 sorry for the mess.

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2011-03-22 at 06:28 PM


(No comment entered for this change.)


Labels: -Milestone-Release09, Milestone-Release10

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2011-03-23 at 01:49 AM


(No comment entered for this change.)


Labels: -Milestone-Release10

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2011-03-26 at 05:40 PM


Just an idea we're kicking around (not a promise):

public abstract class Converter<A, B> extends Function<A, B> {
  protected Converter() {}

  // I don't love these names. The requirement is that converting
  // forward and backward should get you back to a "similar" value,
  // but strict invertibility is too much to require.
  protected abstract B convertForward(A a);
  protected abstract A convertBackward(B b);

  @Nullable public final B apply(@Nullable A a) {
    return (a == null) ? null : convertForward(a);
  }

  public final Converter<B, A> inverse() { ... }

  public final Converter<A, C> compose(Converter<B, C> next) { ... }

  public final Iterable<B> convertAll(Iterable<? extends A> inputs) { ... }

  // also suggested, but... these are just slight conveniences for e.g.
  // Lists.transform() anyway...
  public final Collection<B> convertAll(Collection<? extends A> inputs) { ... }
  public final List<B> convertAll(List<? extends A> inputs) { ... }
}

public final class Converters {
 public static <T> Converter<T, T> identity() { ... }
}

Note that we always convert null to null. We believe this is easier for everyone than having it always be up to individual converter interpretation. And after all, "I don't know how many degrees F it is" is certainly equivalent to "I don't know how many degrees C it is".

We could add these to, e.g., common.primitives.Ints:

  public static Converter<String, Integer> stringConverter() { ... }

We could consider an InvertibleConverter (doesn't that name just roll off the tongue?) subtype that has the much stricter requirement that whenever 'B b = c.convertForward(a)' does not throw an exception, 'c.convertBackward(b).equals(a)' must be true. This doesn't necessarily expose any more functionality, it just helps to caution against assumptions that garden-variety Converters have this property, which so very very many will not.

For example:

  // convertBackward uses name(), not toString()
  public static <E extends Enum<E>> InvertibleConverter<String, E>
      stringConverter(Class<E> enumClass) { ... }

Comments?

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by [email protected] on 2011-04-06 at 01:11 AM


Here's another suggestion at a top-level interface (which Converter might implement as a convenience for certain types of conversions).

public interface ReversibleFunction<A, B> extends Function<A, B> {
    Function<? super B, ? extends A> reverse();
}

Note the capture-of (?)s in the reverse() signature here. An abstract Converter class with an exact one-to-one type equivalence could easily override this with an exact Function<B, A> signature, but it leaves implementers the choice to use a Function with a slightly different signature for the reverse.

The signature of reverse() here also deliberately returns solely Function, not ReversibleFunction. Besides the fact that this is required with the capture-of's, it (1) might not be guaranteed that you could reverse().reverse() and get exactly the same results, or (2) might be that the ReversibleFunction has a complicated implementation, but the reverse of it is trivial with a slightly different signature (say, from a singleton).

Sample combiner utility method that could use the above:

public static <A, B> ReversibleFunction<A, B> join(final Function<A, B> forward, final Function<? super B, ? extends A> reverse) {
    return new ReversibleFunction<A, B>() {
        public B apply(A a) {
            return forward.apply(a);
        }

    public Function<? super B, ? extends A> reverse() {
        return reverse;
    }
};

}

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2011-04-06 at 03:27 AM


Issue #457 has been merged into this issue.

@gissuebot
Copy link
Author

gissuebot commented Oct 31, 2014

Original comment posted by [email protected] on 2011-04-06 at 07:59 AM


I like the idea of a two-way converter between enums and their name representation.

Not sure about the other details, though (but I trust you on that).

@gissuebot
Copy link
Author

Original comment posted by finnw1 on 2011-05-07 at 12:07 AM


I wonder if the "almost-but-not-quite-a-bijection" problem could be mitigated by habitually using a class like this one during development:

class CheckedConverter<A, B> extends ForwardingConverter<A, B> implements InvertibleConverter<A, B> {
    protected B convertForward(A a) {
        B b = delegate().convertForward();
        // Ensure that the conversion is reversible for this value
        assert (convertBackward(b).equals(a)); // Or maybe use a method from Preconditions?
        return b;
    }
    protected A convertBackward(B b) ... // similar
}

With a public entry point here:

public class Converters {
    // ...
    public static <A, B>Converter<A, B> checkedConverter(Converter<A, B> c) {...}
    // And maybe:
    public static <A, B>Converter<A, B> checkedConverter(Converter<A, B> c, Equivalence<A> e1, Equivalence<B> e2) {...}
    // ...
}

I expect I would often find this early warning useful, even if I did not intend to implement InvertibleConverter in my public API.

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2011-07-13 at 06:18 PM


(No comment entered for this change.)


Status: Triaged

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2011-07-16 at 08:37 PM


(No comment entered for this change.)


Status: Accepted

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2011-10-05 at 05:19 AM


(No comment entered for this change.)


Labels: Milestone-Release11

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2011-11-16 at 07:33 PM


(No comment entered for this change.)


Labels: -Milestone-Release11

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2011-12-10 at 03:38 PM


(No comment entered for this change.)


Labels: Package-Base

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2012-02-16 at 06:45 PM


This is being worked on but may miss Guava 12.

@gissuebot
Copy link
Author

Original comment posted by wasserman.louis on 2012-02-16 at 07:07 PM


(No comment entered for this change.)


Blocking: #603

@gissuebot
Copy link
Author

Original comment posted by wasserman.louis on 2012-05-03 at 04:17 PM


(No comment entered for this change.)


Blocking: #985

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2012-05-30 at 07:43 PM


(No comment entered for this change.)


Labels: -Type-Enhancement, Type-Addition
Blocking: -#603, -#985, #603, #985

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2012-11-07 at 07:39 PM


(No comment entered for this change.)


Labels: Milestone-Release14

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2013-01-29 at 06:28 PM


(No comment entered for this change.)


Owner: kurt.kluever
Labels: -Milestone-Release14, Milestone-Release15

@gissuebot
Copy link
Author

Original comment posted by [email protected] on 2013-12-19 at 06:59 PM


75fda2a


Status: Fixed
Labels: -Milestone-Release15, Milestone-Release16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants