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

[5.1] Fix Model forceDelete return type doc #9624

Closed
wants to merge 1 commit into from
Closed

[5.1] Fix Model forceDelete return type doc #9624

wants to merge 1 commit into from

Conversation

stevebauman
Copy link
Contributor

Applied same return type and throws doc as the delete() method.

Applied same return type and throws doc as the `delete()` method.
@stevebauman stevebauman changed the title Fix Model forceDelete return type doc [5.1] Fix Model forceDelete return type doc Jul 14, 2015
@GrahamCampbell
Copy link
Member

I'm pretty sure void was intentional here. It indicates the return value should not be relied on.

@stevebauman
Copy link
Contributor Author

If that's the case, then the delete() method should also indicate the return result void, should it not?

A value is being returned from the delete() method which the forceDelete() returns.

https://github.com/stevebauman/framework/blob/patch-1/src/Illuminate/Database/Eloquent/Model.php#L1119

@GrahamCampbell
Copy link
Member

No? I'm not sure you understand? We're saying you CAN rely on the return type of delete, but not of forceDelete.

@GrahamCampbell
Copy link
Member

That means we can change forceDelete to whatever we want it to return in the future and it's not a breaking change.

@stevebauman
Copy link
Contributor Author

If this is the case, then why would you have a return type on the delete() method? Also why would a forceDelete() method ever return anything more than a boolean?

I'm failing to see what sort of other information would be returned to a developer if the implementation ever needed to change? And if this did change, wouldn't it be listed as a breaking change and have the return type modified just like all other methods?

@GrahamCampbell
Copy link
Member

Also why would a forceDelete() method ever return anything more than a boolean?

We're not saying it doesn't return a bool. We're saying that it's return type shouldn't be relied upon.

Why is that so hard to understand?

@stevebauman
Copy link
Contributor Author

Through the return type listed as void on the method itself, yes, you're saying it returns nothing to anyone who has an IDE.

But, ok 👍

@GrahamCampbell
Copy link
Member

you're saying it returns nothing to anyone who has an IDE.

NO! I'm not saying it returns nothing. I'm saying it returns ANYTHING.

@GrahamCampbell
Copy link
Member

Ie, UNDEFINED.

@stevebauman
Copy link
Contributor Author

Then the return type should be mixed, not void.

@lukasgeiter
Copy link
Contributor

I guess the reason for this choice is that the real forceDelete() in the SoftDeletes Trait actually returns nothing (which means the return value is null)

@GrahamCampbell
Copy link
Member

void DOES NOT mean mixed and DOES NOT mean null.

null means you really must return nothing.
mixed means there are multiple meaningful return values.
void means anything returned should be treated as meaningless.

@stevebauman
Copy link
Contributor Author

The void type, in several programming languages derived from C and Algol68, is the type for the result of a function that returns normally, but does not provide a result value to its caller.

https://en.wikipedia.org/wiki/Void_type

@lukasgeiter
Copy link
Contributor

Not in PHP though... http://php.net/manual/en/language.pseudo-types.php

void as a return type means that the return value is useless.

@stevebauman
Copy link
Contributor Author

Ok 👍

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

Successfully merging this pull request may close these issues.

3 participants