Skip to content

Removed form->isSubmitted() checks #2

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

Closed

Conversation

mickaelandrieu
Copy link

$form->isValid() already check if a form has been validated or not.

So I think it's not required to do this check (see: https://github.com/symfony/Form/blob/master/Form.php#L769)

@weaverryan
Copy link
Member

@mickaelandrieu You're right, but we actually recommend that people add the isSubmitted(). The reason is that isValid() doesn't look like it checked if the form was submitted. So if you're not familiar with Symfony forms, adding isSubmitted() reads more clearly. This detail is hiding inside of the best practices :).

@javiereguiluz
Copy link
Member

@mickaelandrieu thanks for your proposal. It's true that technically speaking this method is unneeded. But for the best practices book it was decided to use ->isSubmitted() in the form recomendations because it feels more natural for Symfony newcomers.

Sadly, that's the reason why I must close this PR without merging it.

@weaverryan
Copy link
Member

@javiereguiluz it looks like we were typing at the same moment!

@javiereguiluz
Copy link
Member

@weaverryan we almost said the same thing at the very same moment in time in completely different parts of the world and without any previous coordination. How weird is that? 😄

@mickaelandrieu
Copy link
Author

@javiereguiluz @weaverryan ok, still sounds very weird to me :/

Why just not say clearly that a form is valid when:

  • the form has been submitted
  • the form has no validation errors
  • this form is not disabled

This could be added directly on Best Practices part.

Anyway, I think symfony-demo is a very good initiative, thank you for this work 👍

@mickaelandrieu mickaelandrieu deleted the removed-is-submitted branch March 24, 2015 14:52
@javiereguiluz
Copy link
Member

@mickaelandrieu it may sound weird at first, but it you read the code as if it were English, you'll notice the following:

  • With isSubmitted(): "if the form is submitted and valid, do this. Otherwise, do that." OK, it makes sense.
  • Without isSubmitted(): "if the form is valid, do this. Otherwise do that." OK, but when do I send the form or process the sent form?

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