From c1fe42fede5682710ad7b727f49603bcdfe1b950 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 31 Aug 2018 23:48:34 +0100 Subject: [PATCH] Hygiene: Use early returns in a few places for special cases When handling special cases that are logically distinct from the function's main branch, it improves code quality (through readability and maintainability) to place those first and with an early return. The has the benefit of the main return statement being easy to find at the end of the function. (Not early and/or in a block). It also means when working on the code, there is generally a less complexity and fewer nesting levels, given that most code is in the main branch. This makes is easier and quicker to verify that code does what it should, as well as making it easy to extend in the future. When considering to add code to end of a function's main scope, it should relate to the function's main branch by default, not a special case. For example, a getName() method should not end with a top-level statement 'return false' (unless it is a stub). Rather, one would expect it to end with `return name`. Change-Id: I1f3088f2409c82dd3bf757fc8fa27dc97ae2767b --- resources/skins.minerva.editor/init.js | 7 ++++--- resources/skins.minerva.scripts/init.js | 10 ++++----- resources/skins.minerva.scripts/pageIssues.js | 21 +++++++++---------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/resources/skins.minerva.editor/init.js b/resources/skins.minerva.editor/init.js index 5a670ac..297ef75 100644 --- a/resources/skins.minerva.editor/init.js +++ b/resources/skins.minerva.editor/init.js @@ -152,9 +152,8 @@ // visualEditorDefault = veConfig && veConfig.defaultUserOptions && veConfig.defaultUserOptions.enable; // return visualEditorDefault ? 'VisualEditor' : 'SourceEditor'; return 'SourceEditor'; - } else { - return preferredEditor; } + return preferredEditor; } /** @@ -416,7 +415,9 @@ // Only load the wikitext editor on wikitext. Otherwise we'll rely on the fallback behaviour // (You can test this on MediaWiki:Common.css) ?action=edit url (T173800) return; - } else if ( !isEditingSupported ) { + } + + if ( !isEditingSupported ) { // Editing is disabled (or browser is blacklisted) updateEditPageButton( false ); showSorryToast( mw.msg( 'mobile-frontend-editor-unavailable' ) ); diff --git a/resources/skins.minerva.scripts/init.js b/resources/skins.minerva.scripts/init.js index 9c388b7..3106a1d 100644 --- a/resources/skins.minerva.scripts/init.js +++ b/resources/skins.minerva.scripts/init.js @@ -133,15 +133,15 @@ // This means MultimediaViewer has been installed and is loaded. // Avoid loading it (T169622) return $.Deferred().reject(); - } else if ( mw.loader.getState( 'mobile.mediaViewer' ) === 'ready' ) { + } + if ( mw.loader.getState( 'mobile.mediaViewer' ) === 'ready' ) { // If module already loaded, do this synchronously to avoid the event loop causing // a visible flash (see T197110) return makeImageOverlay( title ); - } else { - return loader.loadModule( 'mobile.mediaViewer' ).then( function () { - return makeImageOverlay( title ); - } ); } + return loader.loadModule( 'mobile.mediaViewer' ).then( function () { + return makeImageOverlay( title ); + } ); } // Routes diff --git a/resources/skins.minerva.scripts/pageIssues.js b/resources/skins.minerva.scripts/pageIssues.js index dabe661..835a7a4 100644 --- a/resources/skins.minerva.scripts/pageIssues.js +++ b/resources/skins.minerva.scripts/pageIssues.js @@ -218,19 +218,18 @@ * @return {jQuery.Object[]} array of all issues. */ function getIssues( section ) { - if ( section === KEYWORD_ALL_SECTIONS ) { - // Note section.all may not exist, depending on the structure of the HTML page. - // It will only exist when Minerva has been run in desktop mode. - // If it's absent, we'll reduce all the other lists into one. - return allIssues.all || Object.keys( allIssues ).reduce( - function ( all, key ) { - return all.concat( allIssues[key] ); - }, - [] - ); - } else { + if ( section !== KEYWORD_ALL_SECTIONS ) { return allIssues[section] || []; } + // Note section.all may not exist, depending on the structure of the HTML page. + // It will only exist when Minerva has been run in desktop mode. + // If it's absent, we'll reduce all the other lists into one. + return allIssues.all || Object.keys( allIssues ).reduce( + function ( all, key ) { + return all.concat( allIssues[key] ); + }, + [] + ); } /**