Check: Excessive indention
Opened this issue · 5 comments
I'd like to have elm-analyse alert me if I use an "excessive" (configurable?) amount of indentation. For example, here's a snippet of horrific code from work:
viewExpandedBomLine privileges { addNewPart, loadSourceEditor, toMsg } model =
let
{ bomLine, partSearch, confirmingConsignment, quoteSolution } =
model
{ id, symbolDrawing, symbol, footprintDrawing, footprint, substitutes } =
bomLine
{ placements, partType, value } =
bomLine
selectedSubstitute =
bestSubstitute model
partCell =
Html.map toMsg <|
td [ onClick Collapse, colspan 3 ]
(let
header headerText =
Header.h3
(Header.init
|> Header.attached (Just SemanticUI.AttachTop)
|> Header.textAlignment SemanticUI.Centered
|> Header.attributes [ discardClicks ]
|> Header.text (text headerText)
)
in
List.concat
[ [ let
segment =
Segment.segment
(Segment.init
|> Segment.textAlignment SemanticUI.Centered
)
in
div [ class "ui horizontal segments top attached", discardClicks ]
[ segment [ viewSymbol (Maybe.or value symbol) symbolDrawing ]
, segment
[ viewFootprint
{ footprint = footprint
, footprintDrawing = footprintDrawing
, selectedSubstitute = selectedSubstitute
}
]
]
, div []
[ Button.button
(Button.init
|> Button.icon (Just Icon.Search)
|> Button.attributes [ onClick SearchByPackage ]
|> Button.fluid True
|> Button.attached (Just SemanticUI.AttachTop)
)
[ text "Search by package" ]
]
]
, if privileges == Admin then
[ header "Part Type"
, div
[ class "ui buttons bottom attached"
, discardClicks
]
(let
partTypeButton t =
Button.button
(Button.init
|> Button.active (partType == t)
|> Button.attributes [ onClick (SetPartType t) ]
)
[ text <|
case t of
Normal ->
"Normal"I've truncated it, but this clearly needs to be split up. Having some warnings that code is becoming too indented would be a good indicator to act early and start breaking things apart.
Any thoughts?
@ocharles Thanks for your issue. In #10 I already mention Functions where 'too much' happens, but I had a hard time to actually define this. Functions with too much indentation may be a nice subset of function where there is too much going on.
In question that I would like to get answered is: are there cases where you do have a high indent? For example static HTML values with a lot of nested divs and no logic (think about a Bootstrap menu).
I see a lot of let statements being used in other let statements (which is a kind of scoping). Would this be a sufficient check to not allow nested lets? Reducing this may already increase the quality of the function.
Yea, I agree - "too much" is hard to define, but I think indentation is a good measure.
I think nested lets aren't necessarily a smell, I usually end up with excessive indentation when I start nesting case statements directly into HTML trees. But even big HTML trees can have a lot of indentation - this is usually an indicator that the tree should be split into something more abstract, with different sections moved to let bindings.
I agree.
Defining the golden max-indentation is something I do not want to burn my fingers on. Would it be a good option to only check this if for example a maxIndentation property is specified in the elm-analyse.json? Then each project can specify its max, not having people report issues on elm-analyse that the default should be X or Y. I do like defaults, but maximums for these kind of things are always non-trivial context based values.
That sounds perfectly reasonable to me. The community may well discover a common parameter.
The community may well discover a common parameter.
That would be the ultimate goal