Attribute Value Generation: Save In The Method Or Not?

Another design question:

Some of my models have methods to determine a value. For example: my "Invitation" model has a column called "token" which needs to be filled with a unique string at a certain point in time.

There is some business logic to determine this token (generating random strings in a while loop to check if the string does not yet exist in the database), so I created a method in my "Invitation" model.

Now I see two ways to implement this method.

The first option is that the method generates a value and returns it. In that case, the calling code should take care of saving the model:




$invitation = Invitation::findOne(1);

$invitation->token = $invitation->generateToken();

$invitation->save();



The alternative is to make a method that populates the field and saves the model itself. The calling code would be:




$invitation = Invitation::findOne(1);

$invitation->generateToken();



The method would then look like:




public function generateToken()

{

    $token = '';

    //logic to generate the token

    $this->token = $token;

    return $this->save();

}



Please don’t think too much about this specific case (the token). I’m more curious about the conventions you guys use, and any pros and cons.

The second way (where the method also saves the model) is very short and elegant to call, while the other is more flexible, but also needs more manual steps.

I assume it just depends on the majority of the contexts in which the method will be called, but even then I’m eager to know if there is any naming convention to distinguish between methods that save their own model and those that just return a value to be saved into the model.

I lean towards #1 but this ends up looking like procedural code in an OOP wrapper.

However, for #2 it is not clear from the method name or signature that this is actually saving a token as a part of a record. I wonder if I would have answered differently if you had named the method saveToken($invitationId);

I did not think of your third idea. A static method that would do the findOne() itself… interesting!

I think the second way is better because it’s based on “fat model,skinny controller”