iilyak commented on a change in pull request #3766:
URL: https://github.com/apache/couchdb/pull/3766#discussion_r821641763
##########
File path: src/smoosh/src/smoosh_channel.erl
##########
@@ -342,10 +342,27 @@ write_to_file(List, FileName) ->
couch_log:notice("~p Writing state to state file ~s", [?MODULE, FileName]),
OnDisk = <<?VSN, (erlang:term_to_binary(List, [compressed, {minor_version,
1}]))/binary>>,
TmpFileName = FileName ++ ".tmp",
- file:delete(TmpFileName),
- file:write_file(TmpFileName, OnDisk, [sync]),
- file:delete(FileName),
- file:rename(TmpFileName, FileName).
+ case file:delete(TmpFileName) of
Review comment:
I can see following options to reduce nesting level.
# Abstract function
```
file_delete(Path) ->
case file:delete(Path) of
Ret when Ret =:= ok; Ret =:= {error, enoent} ->
ok;
{error, Reason} ->
{error, {Reason, Path}}
end.
write_to_file(List, FileName) ->
...
ok = file_delete(TmpFileName),
ok = file:write_file(TmpFileName, OnDisk, [sync]),
ok = file_delete(FileName),
ok = file:rename(TmpFileName, FileName)
```
Cons: The filename will not be printed on errors from write_file or rename.
# function wrap
```
can_enoent(Fun) ->
case Fun() of
Ret when Ret =:= ok; Ret =:= {error, enoent} ->
ok;
Error ->
Error
end.
log_error(_ErrorMsg, ok) ->
ok;
log_error(ErrorMsg, Error) ->
couch_log:error(ErrorMsg ++ "~p", [Error]),
Error.
TmpFileDeleteError = io_lib:format("~p Unable to delete temporary file ~p",
[?MODULE, TmpFileName]),
ok = log_error(TmpFileDeleteError, can_enoent(fun() ->
file:delete(TmpFileName) end),
```
# write `write_file` function in `smoosh_utils.erl`
I remember seeing the same sequence of commands at least two times. We can
write a generic function once and reuse it.
```
write_file(Module, What, Content) ->
... content of you function almost as is ...
... the difference is in how we log messages ...
couch_log:notice("~p Successfully written ~p to state file ~p", [Module,
What, FileName]);
```
Cons: Level of nesting stays the same
# try/catch/throw pattern
```
file_delete(Path) ->
case file:delete(Path) of
Ret when Ret =:= ok; Ret =:= {error, enoent} ->
ok;
Error ->
Error
end.
throw_on_error(_Args, ok) ->
ok;
throw_on_error(Args, {error, Reason}) ->
throw({error, {Reason, Args}}).
write_to_file(List, FileName) ->
...
try
throw_on_error(TmpFileName, file_delete(TmpFileName)),
throw_on_error(TmpFileName, file:write_file(TmpFileName, OnDisk,
[sync])),
throw_on_error(FileName, file_delete(FileName)),
throw_on_error([TmpFileName, FileName], file:rename(TmpFileName,
FileName)),
of
catch
throw:Error -> Error
end.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
