-
Notifications
You must be signed in to change notification settings - Fork 10
Add guidelines on judicious use of recursion #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
-module(recursion). | ||
|
||
-export([recurse/1, fold/1, map/1, comprehension/1]). | ||
|
||
%% | ||
%% Example: | ||
%% Different functions to capitalize a string | ||
%% | ||
|
||
%% BAD: makes unnecessary use of manual recursion | ||
recurse(S) -> | ||
lists:reverse(recurse(S, [])). | ||
|
||
recurse([], Acc) -> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if this is meant as an illustration of your point (if so 👍 😄 ), but this doesn't work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hahaha I wish I could claim credit for intentionally writing a bug in this function, but admittedly I did not actually test the code since I was just trying to convey the gist of the idea, and didn't expect anyone to actually run it 😆 . I will fix it along with addressing your other comment above, and then do a rebase and push. |
||
Acc; | ||
recurse([H | T], Acc) -> | ||
NewAcc = [string:to_upper(H) | Acc], | ||
recurse(T, NewAcc). | ||
|
||
%% GOOD: uses a fold instead to achieve the same result, | ||
%% but this time more safely, and with fewer lines of code | ||
fold(S) -> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lists map There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid point! I had really wanted to demonstrate using a fold in my example, since almost any recursive list iteration function can be converted to a fold, so perhaps I will add a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
Result = lists:foldl(fun fold_fun/2, [], S), | ||
lists:reverse(Result). | ||
|
||
fold_fun(C, Acc) -> | ||
[string:to_upper(C) | Acc]. | ||
|
||
%% BETTER: uses a map instead of a fold to yield a simpler | ||
%% implementation, since in this case a fold is overkill | ||
map(S) -> | ||
lists:map(fun string:to_upper/1, S). | ||
|
||
%% BEST: in this case, a list comprehension yields the | ||
%% simplest implementation (assuming we ignore the fact | ||
%% that string:to_upper can also be used directly on strings) | ||
comprehension(S) -> | ||
[string:to_upper(C) || C <- S]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went to go play with this and noticed that the functions aren't exported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, good call. I semi-deliberately left out the exports since I was mainly intending for this code to be a concise, illustrative example, rather than something anyone might ever actually want to run. I suppose it'd be easy enough to just add the one export line though and have a working module, so I'll go ahead and do that.