It’s deeply nested expressions. I’m not writing it with EMACS or other IDEs, but with my own editor. So it might looks strange.
The thing got me worried is I never wrote code nested in so many levels before. It’s fine in my own way. But what others would think about my code? Is such code common in the community?
The logics in the snippets is to group tasks by year/week/day and then render them in lists. I used group-by for 3 times.
I believe there’s rarely a reason to write a function that doesn’t fit in one screen (with a very large font that is!). Also, there’s no reason to have lots of anonymous functions in let blocks - just define them as a normal defns (same applies to almost any anonymous fn that is longer than one line). If you have more than 2-3 levels of nesting - split it in several functions. Ideally, all your fns should look like (->> val (foo …) (bar …) (baz …)), maybe with a let around it - so (almost) no nesting, everything is as flat and linear as possible.
(Of course, this is just my personal opinion, YMMV.)
Your editor seems to have quite wide whitespacing alignment, which probably makes it look a lot wider. That said, you could benefit from factoring some parts out to their own functions.
I considered about refactoring. However it’s UI code, I don’t see how it will be reused. I think maybe keeping it the way it is, corresponding to its DOM structure, would make it easier to read?
Ya, after looking at the github link instead, where whitespace is much more compact. I think in your case its probably fine. A big nested query, you might not need to share the sub-parts of it.
Moving it into sub-functions could maybe allow testing the sub-queries, but it might be overkill, better spend your time testing other things.
I’d say, if the nesting was so deep, that my screen needs to scroll horizontally to see it all, I would personally break them apart just so to avoid having to scroll that way. But, if you don’t mind that, its not that there’s much wrong with it, except, at quick glance, its a bit hilarious looking.
As others have said I would recommend breaking things apart into smaller functions. Regardless of whether they will “reused” or not. It just makes things easier to read and test.
Also consider using for instead of map.
(for [[year tasks-by-week] tasks-by-year]
...)
looks nicer than
(->> tasks-by-year
(map (fn [[year tasks-by-week] ...))
I haven’t used for before in my code. Look good in my case!
Impressive difference https://github.com/Memkits/pudica-schedule-viewer/pull/7/commits/cb74b6599a6d9318411fee7e56490bf26d7ae1f1
This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.