Well-written as opposed to working but poorly written code is a distinct matter of opinion and taste. There are various style guides if you’re interested in seeing examples.
Personally in your example above, I would change two things: indentation, and curly braces.
I find php open/closing tags in and html tags in templates to be hard to read when they are not lined up with the rest of the indentation:
<?php
if ( ! is_user_logged_in() ) {
$args = array(
'redirect' => site_url( "https://wordpress.stackexchange.com/index.php/portal" ),
'form_id' => 'loginform-custom',
'label_username' => __( 'Username' ),
'label_password' => __( 'Password' ),
'label_remember' => __( 'Remember Me' ),
'label_log_in' => __( 'Log in' ),
'remember' => true
);
wp_login_form( $args );
?>
<p>The portal is for registered users only.</p>
<?php
} else {
wp_loginout( index.php/portal() );
?>
<p>We recommend logging out after each session.</p>
<p><a href="https://wordpress.stackexchange.com/index.php/portal">Continue to portal</a></p>
<?php } ?>
This makes it much clearer what goes inside each if/else block. Along the same vein, I would replace the if/else curly braces with colons and endif tags, as well as move any php open/close tags onto the same lines as the php functionality.
<?php if ( ! is_user_logged_in() ) : ?>
<?php
$args = array(
'redirect' => site_url( "https://wordpress.stackexchange.com/index.php/portal" ),
'form_id' => 'loginform-custom',
'label_username' => __( 'Username' ),
'label_password' => __( 'Password' ),
'label_remember' => __( 'Remember Me' ),
'label_log_in' => __( 'Log in' ),
'remember' => true
);
wp_login_form( $args ); ?>
<p>The portal is for registered users only.</p>
<?php else: ?>
<?php wp_loginout( index.php/portal() ); ?>
<p>We recommend logging out after each session.</p>
<p><a href="https://wordpress.stackexchange.com/index.php/portal">Continue to portal</a></p>
<?php endif; ?>
This makes it easier to see which if/else/end blocks are related to each other, as well ask making it clear what functionality is php and what is HTML.
Multi-line php blocks are somewhat of a code smell to me, indicating logic that should be put in a function or class elsewhere and referenced in a single line in your template:
<?php
# some other file, maybe have a modules/ or helpers/ directory in your plugin
# or theme that contains functions and classes that are used for functionality.
function 5south_custom_login_form() {
$args = array(
'redirect' => site_url( "https://wordpress.stackexchange.com/index.php/portal" ),
'form_id' => 'loginform-custom',
'label_username' => __( 'Username' ),
'label_password' => __( 'Password' ),
'label_remember' => __( 'Remember Me' ),
'label_log_in' => __( 'Log in' ),
'remember' => true
);
wp_login_form( $args );
}
# your template file
<?php if ( ! is_user_logged_in() ) : ?>
<?php 5south_custom_login_form() ?>
<p>The portal is for registered users only.</p>
<?php else: ?>
<?php wp_loginout( index.php/portal() ); ?>
<p>We recommend logging out after each session.</p>
<p><a href="https://wordpress.stackexchange.com/index.php/portal">Continue to portal</a></p>
<?php endif; ?>
Generally I find that if there’s more than 5-10 lines in a php if block in a template file, its usually worth breaking that functionality into a separate template and including it inside the if or else statement. This makes the template logic easier to follow.
If you find yourself nesting if/else statements in a complex way in a template file, it’s probably time to reassess how the template file is loaded and perhaps create a function or functions to choose exact templates or partial templates for each specific situation.
Some people will see what I’ve done above and throw up in their mouth a little.