Find an elegant way to show that a redirect stop the method flow

[ia3andy]

Currently, it works like this:

    @POST
    public void add(@RestForm @NotBlank String title, @RestForm @NotBlank String note) {
        // If validation fails, redirect to the todos page (with errors propagated)
        if(validationFailed()) {
            // redirect to the todos page by just calling the method: it does not return!
            todos();
        }
        // save the new Todo
        Note todo = new Note();
        todo.task = task;
        todo.persist();
        // redirect to the todos page
        todos();
    }

Some could argue that it is not obvious in a java world that todos(); is stopping the flow of the method. It would be nice to offer an alternate more declarative way of doing it. Something like 👇 would make me happy :) :

    @POST
    public void add(@RestForm @NotBlank String title, @RestForm @NotBlank String note) {
        if(validationFailed()) {
            redirect(Todos::todos);
			return;
        }

        Note todo = new Note();
        todo.task = task;
        todo.persist();

        redirect(Todos::todos);
    }

[cmoulliard]

redirect(Todos::todos);

If validation fails, that makes sense to redirect at the condition that you redirect to perhaps another page as here redirect is used no matter if it succeeds or fails --> redirect(Todos::todos);

[ia3andy]

Mind that with the suggested syntax, there is no need for comments anymore, it's self explainatory.

[ia3andy]

redirect(Todos::todos);

If validation fails, that makes sense to redirect at the condition that you redirect to perhaps another page as here redirect is used no matter if it succeeds or fails --> redirect(Todos::todos);

I don't think that the page you redirect to is changing the issue. Whether you redirect to the same page or not, I am saying it should show expressly that the flow is stopped with a return.

You are right though on the fact that if the validation failed, you might want to add some error information. So maybe a throw would be more relevant in that case, something like:

    @POST
    public void add(@RestForm @NotBlank String title, @RestForm @NotBlank String note) {
        if(validationFailed()) {
            throw new RedirectException(Todos::todos, getValidationErrors());
        }

        Note todo = new Note();
        todo.task = task;
        todo.persist();

        redirect(Todos::todos);
    }

[cmoulliard]

Mind that with the suggested syntax, there is no need for comments anymore, it's self explainatory.

For sure but I suggest then something like this as example ;-)

@POST
    public void add(@RestForm @NotBlank String title, @RestForm @NotBlank String note) {
        if(validationFailed()) {
            redirect(Todos::wrongFormTodo);
			return;
        }

        Note todo = new Note();
        todo.task = task;
        todo.persist();

        redirect(Todos::todos);
    }

[cmoulliard]

This syntax is even better

        if(validationFailed()) {
            throw new RedirectException(Todos::todos, getValidationErrors());
        }

Question:

• Will the errors be passed to the Qute Template using {errors} ?
• Will the errors generated as a Map where the key will match the erroneous field ?

[FroMage]

        throw new RedirectException(Todos::todos, getValidationErrors());

is what: todos() does, but with lots more boilerplate ;)

I understand your initial reaction about this, but give the current redirect feature a try, I swear it will grow on you to the point where you never want to use anything else ;)

[mkouba]

Hm, and this one?

   @POST
    public void add(@RestForm @NotBlank String title, @RestForm @NotBlank String note) {
        // If validation fails, redirect to the todos page (with errors propagated)
        if (!validationFailed()) {
           // save the new Todo
           Note todo = new Note();
           todo.task = task;
           todo.persist();
        }
        // redirect to the todos page
        todos();
    }

[FroMage]

That's fine too, but naive in most scenarios, because quite often, there's more calls to validationFailed() as validation rules are added as new conditions. Also, it doesn't actually solve their issue of showing that todos() is a throwing method.

[ia3andy]

Hm, and this one?

   @POST
    public void add(@RestForm @NotBlank String title, @RestForm @NotBlank String note) {
        // If validation fails, redirect to the todos page (with errors propagated)
        if (!validationFailed()) {
           // save the new Todo
           Note todo = new Note();
           todo.task = task;
           todo.persist();
        }
        // redirect to the todos page
        todos();
    }

when you have a problem, the solution is to remove the problem! clever boy 😊

Still this is a different coding style and I think allowing the if at the top way of doing things is important.

is what: todos() does, but with lots more boilerplate ;)

I understand your initial reaction about this, but give the current redirect feature a try, I swear it will grow on you to the > point where you never want to use anything else ;)

@FroMage TBH I am fine with the syntax and could live happily ever after with it. Still I think a lot of people might turn around because of this kind of thing. So better give them a way in :)

[ia3andy]

@FroMage FYI @mcruzdev had the same feeling I had on this one

[mr-mos]

Totally agree with @ia3andy
Another strong argument beside that a Java programmer can't understand this without reading documentation or go deep into the framework. Using

if (validationFailed()) {
            editBlogEntry(id);
           // Redirect to this blog entry (no need to return, this will stop here because of an internal exception)
}

always leads to a redirect. In my application and many others you don't do a redirect in case of an error, but instead just render the page (editBlogEntry) without a redirect. A redirect leads to bad usability.
You loose you form-data after a refresh (flash is just holding it for the first redirect).

The rule of thumb is: Do a redirect in case of success and just render the page in case of an error

[FroMage]

I disagree with that statement, especially in Renarde, we always redirect back to the form, preserving the values, in case of errors. That's the most useful thing to do in most cases. Why would you say it's bad usability?

[mr-mos]

If a user submits a form with invalid data and an error occurs, redirecting to the same form may cause all the input fields to be cleared. This forces the user to re-enter their data, which can be frustrating, especially for complex or long forms. Even though techniques like using flash storage can temporarily hold data, it is only available for the first redirect and can be lost if the user refreshes the page, leading to potential data loss.
A better approach in many cases is to render the page with the form again, showing the validation errors while preserving the entered data. This allows users to correct their input without having to re-enter everything, providing a smoother and less frustrating experience.

Best practice:
The best practice regarding handling POST requests is to redirect only after a successful submission but to re-render the page with validation errors if the form fails. This approach is commonly used to prevent data loss and improve user experience.

1. POST-Redirect-GET Pattern (PRG): After a successful form submission, a redirect ensures that users don't accidentally resubmit the form by refreshing the page. This avoids duplication of data and makes for a smoother user experience. See: https://en.wikipedia.org/wiki/Post/Redirect/Get
2. Rendering on Error: If there's an error during submission, it is generally better to re-render the form with the original data, allowing the user to correct mistakes without losing their input. In cases where validation fails, returning the view directly (without a redirect) maintains the entered data and error messages. This is crucial for maintaining usability, especially in large or complex forms.

This pattern of redirecting on success but rendering the form again on failure ensures users don’t lose their input and reduces frustration in cases of validation errors.

Remember also: Some application may choose not to use the web-session at all (because of some architecture decisions like scaling issues; function as a service) They can't even use the flash-context. Meaning the use of Renarde wouldn't work at all.

[ia3andy]

I didn't know about it, but what @mr-mos proposed seems like a better approach

[mr-mos]

This would also solve #192