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

Make dummy entity setters @deprecated #64

Open
yamass opened this issue Oct 7, 2021 · 2 comments
Open

Make dummy entity setters @deprecated #64

yamass opened this issue Oct 7, 2021 · 2 comments

Comments

@yamass
Copy link
Collaborator

yamass commented Oct 7, 2021

I've stumbled across a lot of code where developers have been using dummies and then setting values on them, e.g.:

redG.dummyX()
    .y("value")

We should make sure developers do this only if they know what they are doing, since dummies are usually reused.

So I propose deprecating all setters on dummies with a corresponding JavaDoc comment saying something like "Dummies are fillers that have random 'don't care' values and transitive dummy dependencies (non-null foreign keys). Note that dummies are generally shared, so invoking redG.dummyX() twice will return the same instance. Setting a value on it will affect all code using the dummy. Since setting a value on a dummy generally indicates that you do care about the value, you probably want to addX(...) instead.

@olewehrmeyer How do you think about it?

@olewehrmeyer
Copy link
Collaborator

@yamass sounds like a good idea. In a way, it could also remove the necessity of the DummyFactory.isDummy(RedGEntity) method.

But it would be a bigger feature code-generation-wise, as currently all dummy entities are just of the normal entity class.
In this case, we would either need a whole new DummyGTableName class with a full implementation or a wrapper class that then simply delegates the calls to the wrapped entity.

The wrapping part could be done transparently, so that it would not be a breaking change and could look like this:

    public DummyGTableName dummyTableName() {
        return DummyGTableName.wrap(this.getDummyFactory().getDummy(this, GTableName.class));
    }

This would not require any changes to the DummyFactory implementations of the users.

Without wrapping/delegating stuff, we could change DummyFactory so that

public interface DummyFactory {
  <T extends RedGDummyEntity> T getDummy(AbstractRedG var1, Class<T> var2);
}

and then RedGDummyEntity simply extends RedGEntity and is implemented by DummyGTableName with basically the same code as the normal entities, just with set-methods marked as deprecated.

Do you have a strong preference? I feel like the wrapper would be better as a non-breaking-change and I see no real downsides.

To make the "don't set values on dummy entities" even more explicit in the future, we could do a two-step release, where in the first we mark the normal setter as deprecated and introduce a dummy.explicitelyFieldName(x) setter with a prefixed method name that highlights that what you are doing is technically possible but discouraged. In a second release a bit later we could then remove the deprecated method completely.

@yamass
Copy link
Collaborator Author

yamass commented Oct 12, 2021

@olewehrmeyer
As discussed on the phone:

We agreed on the wrappers option with a slightly different approach.

  1. Generate interfaces for GEntities
  2. Make the standard GEntity classes implement that interface.
  3. Generate dummy wrapper classes that also implement the interface but delegate to a standard instance, marking all setters as @deprecated.
interface GProduct {...}

class GProductImpl implements GProduct {
    // same as before...
}

class GDummyProduct implements GProduct {
    private GProduct delegate;

    ...
   
    /**
     * @deprecated Dummies are fillers that [...] Since setting a value on a dummy generally indicates that you do care about the value, you probably want to addX(...) instead.
     **/
    @Deprecated
    @Override
    public GDummyProduct name(String name) {
        delegate.name(name);
        return this;
    }
}

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

2 participants