Skip to main content
added 775 characters in body
Source Link
TorbenPutkonen
  • 7.9k
  • 20
  • 28

As I mentioned in the comment, you should strive to using composition instead of inheritance. The AbstractVerificationHandler exists only as an extension point for the user to provide access to the data. Instead of implementing fetching and verifing as abstract methods, use suppliers and predicates. For example:

public class VerificationTask<T extends Identifiable, U extends Identifiable> {
    // Provide these in a constructor.
    final Supplier<Collection<T>> supplierT;
    final Supplier<Collection<U>> supplierU;
    final BiPredicate<T, U> verifier;

    public boolean verify() {
        Collection<T> tRecords = supplierT.get();
        Map<String, U> uRecords = identityMap(supplierU.get());
        for (T record: tRecords) {
            if (!verifier.test(record, uRecords.get(record.identity()))) {
                return false;
            }
        }
        return true;
    }
}

The users are loaded with one purpose suppliers:

public class DBUserLoader implements Supplier<Collection<DBUser>> {
    public Collection<DBUser> get() {
        // YourLoad codeusers from AbstractVerificationHandlerDB.verify
    }
}

UserVerifier becomes a simple one purpose predicate:

public class UserVerifier extends BiPredicate<DBUser, FileUser> {
    public boolean test(DBUser dbuser, FileUser fileuser) {
        // fields specific comparison with different names etc. 
    }
}

Following this your code doesn't violate the single responsibility principle. In your code the UserVerifier class is responsible for three things: loading data from DB, loading data from file and comparing the entries.

I renamed the Identify interface as Identifiable as the word "identify" describes a command while "identifiable" describes a capability. It also follows the custom used in the Java standard libraries (e.g. Comparable, Iterable).

You should use as generic collection classes as you canget away with. Requiring a List when the code does not require any list specific operations unnecessarily limits the programmer who implements your API. Maybe they are forced to use another API that provides a Set, which would require them to repack their data to a List before handing it over to you.

As I mentioned in the comment, you should strive to using composition instead of inheritance. The AbstractVerificationHandler exists only as an extension point for the user to provide access to the data. Instead of implementing fetching and verifing as abstract methods, use suppliers and predicates. For example:

public class VerificationTask<T extends Identifiable, U extends Identifiable> {
    // Provide these in a constructor.
    final Supplier<Collection<T>> supplierT;
    final Supplier<Collection<U>> supplierU;
    final BiPredicate<T, U> verifier;

    public boolean verify() {
        // Your code from AbstractVerificationHandler.verify()
    }
}

Following this your code doesn't violate the single responsibility principle. In your code the UserVerifier class is responsible for three things: loading data from DB, loading data from file and comparing the entries.

I renamed the Identify interface as Identifiable as the word "identify" describes a command while "identifiable" describes a capability. It also follows the custom used in the Java standard libraries (e.g. Comparable, Iterable).

You should use as generic collection classes as you canget away with. Requiring a List when the code does not require any list specific operations unnecessarily limits the programmer who implements your API. Maybe they are forced to use another API that provides a Set, which would require them to repack their data to a List before handing it over to you.

As I mentioned in the comment, you should strive to using composition instead of inheritance. The AbstractVerificationHandler exists only as an extension point for the user to provide access to the data. Instead of implementing fetching and verifing as abstract methods, use suppliers and predicates. For example:

public class VerificationTask<T extends Identifiable, U extends Identifiable> {
    // Provide these in a constructor.
    final Supplier<Collection<T>> supplierT;
    final Supplier<Collection<U>> supplierU;
    final BiPredicate<T, U> verifier;

    public boolean verify() {
        Collection<T> tRecords = supplierT.get();
        Map<String, U> uRecords = identityMap(supplierU.get());
        for (T record: tRecords) {
            if (!verifier.test(record, uRecords.get(record.identity()))) {
                return false;
            }
        }
        return true;
    }
}

The users are loaded with one purpose suppliers:

public class DBUserLoader implements Supplier<Collection<DBUser>> {
    public Collection<DBUser> get() {
        // Load users from DB.
    }
}

UserVerifier becomes a simple one purpose predicate:

public class UserVerifier extends BiPredicate<DBUser, FileUser> {
    public boolean test(DBUser dbuser, FileUser fileuser) {
        // fields specific comparison with different names etc. 
    }
}

Following this your code doesn't violate the single responsibility principle. In your code the UserVerifier class is responsible for three things: loading data from DB, loading data from file and comparing the entries.

I renamed the Identify interface as Identifiable as the word "identify" describes a command while "identifiable" describes a capability. It also follows the custom used in the Java standard libraries (e.g. Comparable, Iterable).

You should use as generic collection classes as you canget away with. Requiring a List when the code does not require any list specific operations unnecessarily limits the programmer who implements your API. Maybe they are forced to use another API that provides a Set, which would require them to repack their data to a List before handing it over to you.

added 370 characters in body
Source Link
TorbenPutkonen
  • 7.9k
  • 20
  • 28

As I mentioned in the comment, you should strive to using composition instead of inheritance. The AbstractVerificationHandler exists only as an extension point for the user to provide access to the data. Instead of implementing fetching and verifing as abstract methods, use suppliers and predicates. For example:

public class VerificationTask<T extends Identifiable, U extends Identifiable> {
    // Provide these in a constructor.
    final Supplier<List<T>>Supplier<Collection<T>> supplierT;
    final Supplier<List<U>>Supplier<Collection<U>> supplierU;
    final BiPredicate<T, U> verifier;

    public boolean verify() {
        // Your code from AbstractVerificationHandler.verify()
    }
}

Following this your code doesn't violate the single responsibility principle. In your code the UserVerifier class is responsible for three things: loading data from DB, loading data from file and comparing the entries.

I renamed the Identify interface as Identifiable as the word "identify" describes a command while "identifiable" describes a capability. It also follows the custom used in the Java standard libraries (e.g. Comparable, Iterable).

You should use as generic collection classes as you canget away with. Requiring a List when the code does not require any list specific operations unnecessarily limits the programmer who implements your API. Maybe they are forced to use another API that provides a Set, which would require them to repack their data to a List before handing it over to you.

As I mentioned in the comment, you should strive to using composition instead of inheritance. The AbstractVerificationHandler exists only as an extension point for the user to provide access to the data. Instead of implementing fetching and verifing as abstract methods, use suppliers and predicates. For example:

public class VerificationTask<T extends Identifiable, U extends Identifiable> {
    // Provide these in a constructor.
    final Supplier<List<T>> supplierT;
    final Supplier<List<U>> supplierU;
    final BiPredicate<T, U> verifier;

    public boolean verify() {
        // Your code from AbstractVerificationHandler.verify()
    }
}

Following this your code doesn't violate the single responsibility principle. In your code the UserVerifier class is responsible for three things: loading data from DB, loading data from file and comparing the entries.

I renamed the Identify interface as Identifiable as the word "identify" describes a command while "identifiable" describes a capability. It also follows the custom used in the Java standard libraries (e.g. Comparable, Iterable).

As I mentioned in the comment, you should strive to using composition instead of inheritance. The AbstractVerificationHandler exists only as an extension point for the user to provide access to the data. Instead of implementing fetching and verifing as abstract methods, use suppliers and predicates. For example:

public class VerificationTask<T extends Identifiable, U extends Identifiable> {
    // Provide these in a constructor.
    final Supplier<Collection<T>> supplierT;
    final Supplier<Collection<U>> supplierU;
    final BiPredicate<T, U> verifier;

    public boolean verify() {
        // Your code from AbstractVerificationHandler.verify()
    }
}

Following this your code doesn't violate the single responsibility principle. In your code the UserVerifier class is responsible for three things: loading data from DB, loading data from file and comparing the entries.

I renamed the Identify interface as Identifiable as the word "identify" describes a command while "identifiable" describes a capability. It also follows the custom used in the Java standard libraries (e.g. Comparable, Iterable).

You should use as generic collection classes as you canget away with. Requiring a List when the code does not require any list specific operations unnecessarily limits the programmer who implements your API. Maybe they are forced to use another API that provides a Set, which would require them to repack their data to a List before handing it over to you.

Source Link
TorbenPutkonen
  • 7.9k
  • 20
  • 28

As I mentioned in the comment, you should strive to using composition instead of inheritance. The AbstractVerificationHandler exists only as an extension point for the user to provide access to the data. Instead of implementing fetching and verifing as abstract methods, use suppliers and predicates. For example:

public class VerificationTask<T extends Identifiable, U extends Identifiable> {
    // Provide these in a constructor.
    final Supplier<List<T>> supplierT;
    final Supplier<List<U>> supplierU;
    final BiPredicate<T, U> verifier;

    public boolean verify() {
        // Your code from AbstractVerificationHandler.verify()
    }
}

Following this your code doesn't violate the single responsibility principle. In your code the UserVerifier class is responsible for three things: loading data from DB, loading data from file and comparing the entries.

I renamed the Identify interface as Identifiable as the word "identify" describes a command while "identifiable" describes a capability. It also follows the custom used in the Java standard libraries (e.g. Comparable, Iterable).