Return to Revisions

#The Abstract class Decorator is too light I find your DomainRangeDecorator abstract class a bit lacking in functionality. I understand you made it a backbone for all Domain Ranges, but what functionality does it centralize?

• It embeds the decorated DomainRange as a field (composite pattern, this is good)
• It forwards the getDomain method to that field (so it is not, in fact, abstract!)

If you go through the hassle of creating multiple classes and abstract prototypes, at least make it worthwhile. This class could do more to alleviate the other DomainRange implementations. For instance, you know the decorators will increase the Domain, and you know they'll do it by appending their allowed character set to the decorated character set. So make this 'append' operation centralized:

public abstract class DomainRangeDecorator implements DomainRange {
    private DomainRange domainRange;

    public DomainRangeDecorator(DomainRange domainRange) {
        this.domainRange = domainRange;
    }

    @Override
    public String getDomain() {
        return domainRange.getDomain().concat(additionalDomain());
    }

    protected abstract String additionalDomain();
}

public class UppercaseDomainRange extends DomainRangeDecorator {
    public UppercaseDomainRange(DomainRange domainRange) {
        super(domainRange);
    }

    @Override
    protected String additionalDomain() {
        return Charset.ALPHA.getDomainRange().toUpperCase();
    }
}

This is the point of inheritance: inheriting behaviours, not just avoiding to repeat implementation details. In your implementation, there was no behaviour to speak of.

Additionally, inheritance standardizes the behaviour: before, you had some classes append with + and some with concat. Now it's in one place, and you can enrich it (with a null check for example: don't trust your sub-classes!) and enjoy the added functionality everwhere else .

#The PasswordGenerator is insufficient because the DomainRange lacks functionality

What does PasswordGenerator do right now? It picks chars at random in a list.

This gives no guarantee on the password. I could get abcd even though I prime the charset with:

return new LowercaseDomainRange(new SpecialCharsDomainRange(new UppercaseDomainRange(new NumericDomainRange(new BaseDomainRange()))));

Many website require at least a number, a symbol, a lowerscript etc.

You may well say:

That's OK, I'll add that functionality later, it's just like the Entropy Calculation.

But is it ok, though?

I believe you have a structural problem preventing you from solving the issue. Yes, you can check if the password matches some rules, and those rules could be "must contain a digit" as well as entropy must be >10". But how do you make sure your Generator has generated a password which does match the requested contraints?

You could loop until you find a satisfying password (which you need anyway to satisfy entropy checks), but solving many requirements like this would be inefficient.

Your Decorator structure is hiding away implementations (which is normal). The PasswordGenerator can only access basic DomainRange functionality, which does not include a constraint alleviation method like 'give me a char from your charset alpha/num/symbol. Even if you do expose such a method like getCharInRestrictedCharset(), you won't be able to satisfy them all, because the DomainRange may or may not be a decorator, and the decorated object may or may not be a decorator with constraints, etc, and you cannot access the inner DomainRanges for their own getCharInRestrictedCharset()` method.

By wrapping the charsets like Russian dolls, you've made the inner layers inaccessible. Making them accessible would break encapsulation.

If instead of an onion structure, the charsets were laid out "flat" (each accessible) then the password generator would be availble to pick from whichever charset it wants! Then constructing a password that satisfies constraints would be possible from the get-go.

You could have a List<DomainRange> in the PasswordGenerator. This way it can pick from a specific one to match a constraint, or from a random one.