# Please give feedback on my function
**URL:** https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280
**Category:** New to Julia
**Tags:** function
**Created:** [April 30, 2022, 12:55am UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280 "2022-04-30T00:55:47Z")
**Posts on this page:** 20
**Page:** 1
### Author:  [@chadagreene](https://discourse.julialang.org/u/chadagreene)
#### Post date: [April 30, 2022, 12:55am UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/1 "2022-04-30T00:55:47Z")
I’m very new to Julia, and to get started I’m translating some old MATLAB functions into a package that will be used analyze glacier measurements. Here’s my first attempt at writing a function, and I want to make sure I’m starting out on the right foot.
- Does anything need to be changed to make it more Julia-y?
- Does anything–even the docstring formatting–jump out at you as bad syntax?
Feel free to be brutal in your feedback, but please let me know which suggestions are essential versus which might be minor personal preferences.
````julia
"""
T = freeboard2thickness(F,kwargs)
Estimate ice thickness from surface height above sea level, assuming hydrostatic equilibrium.
# Input
- `F` (m) freeboard surface height above sea level.
- `rho_ice` (kg/m³) density of ice layer (default: `917`).
- `rho_seawater` (kg/m³) density of seawater (default: `1027`).
- `rho_snow` (kg/m³) density of snow layer (default: `350`). Inconsequential unless `T_snow>0`.
- `T_snow` (m) thickness of snow layer (default: `0`).
# Output
- `T` (m) thickness of ice layer.
# Examples
### Example 1: Simple case
The surface of an iceberg is 80 cm above the surrounding ocean surface, and the iceberg is
made of pure ice, without any snow on it. How thick is the iceberg?
```julia
julia> freeboard2thickness(0.8)
7.46909090909091
```
### Example 2: Specifying parameters
The surface of an iceberg is 80 cm above sea level, and has a 40 cm layer of snow on top,
whose density is 300 kg/m³. How thick is the ice portion of this iceberg?
```julia
julia> freeboard2thickness(0.8; T_snow=0.4, rho_snow=300)
4.825454545454546
```
# Author Info
Chad A. Greene, NASA Jet Propulsion Laboratory
April, 2022
"""
```julia
function freeboard2thickness(
F;
rho_ice=917,
rho_seawater=1027,
rho_snow=350,
T_snow=0
)
T = F*rho_seawater/(rho_seawater-rho_ice) - T_snow*(rho_seawater-rho_snow)/(rho_seawater-rho_ice);
return T
end
```
````
---
### Author:  [@suavesito](https://discourse.julialang.org/u/suavesito)
#### Post date: [April 30, 2022, 12:59am UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/2 "2022-04-30T00:59:58Z")
> [@chadagreene](#):
>
> (I’m not sure how to get the text below to _not_ render as markdown on this forum, but the docstring is below, followed by the function.)
Covered by [this](https://discourse.julialang.org/t/psa-how-to-quote-code-with-backticks/7530) PSA.
(Edit: also changed the title, don’t know if _insult_ is the easier way to get people interested. 😅)
---
### Author:  [@suavesito](https://discourse.julialang.org/u/suavesito)
#### Post date: [April 30, 2022, 1:12am UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/3 "2022-04-30T01:12:14Z")
As for your function, it is all right. 🙂
If you want _and_ there is an established well known nomenclature on your area, you can use Unicode for the \rho's and alike, some people like it, some don’t. One thing to favor not using Unicode is that not everybody has a code viewer/font that renders well Unicode, so
For the docs
> [@chadagreene](#):
>
> ```julia
> T = freeboard2thickness(F,kwargs)
>
> ```
don’t just put `kwargs`, you can use the way it is used in Base, this is
```julia
freeboard2thickness(freeboard; rho_ice=917, rho_seawater=1027, rho_snow=350, T_snow=0)
```
and later on explain from _where_ you got those default parameters. 🙂
---
### Author:  [@suavesito](https://discourse.julialang.org/u/suavesito)
#### Post date: [April 30, 2022, 2:16am UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/4 "2022-04-30T02:16:26Z")
This is what I would write for docstrings + function, I did the next changes
- Changed all \rho for density, this way you don’t need to specify the meaning, as it is clear by the name
- change the name of the function, it is more readable for non-native English speakers (I’m one 😅)
- changed keyword arguments to be similar to base syntax of docstrings (you can see `?sort`)
- reduce the examples, as I don’t think they need to be so verbose (not a critic, just my opinion), specially because the keyword arguments are more expressive
Personally, I wouldn’t mind putting the author there, as that information would come in the package registry and license. But does not harm in any way. 🙂
For the code, I just separate it in two lines for it not to be too large. The code itself is alright, I just will mention that maybe you want to use [Unitful.jl](https://github.com/PainterQubits/Unitful.jl) for the units. 😃
````julia
"""
freeboar_to_thickness(freeboard_thickness; ice_density=917, seawater_density=1027, snow_density=350, thickness_snow=0)
Estimate ice thickness (in meters) from surface height above sea level, assuming hydrostatic equilibrium. Thickness are expected to be in meters and densities in kg/m^3.
The default parameters come from (here you put the reference).
# Examples
```
julia> freeboard_to_thickness(0.8) # pure ice, 80cm above sealevel
7.46909090909091
julia> freeboard_to_thickness(0.8; snow_thickness=0.4, snow_density=300) # 80cm above sea level, 40cm of snow with density 300 kg/m^3
4.825454545454546
```
# Author Info
Chad A. Greene, NASA Jet Propulsion Laboratory
April, 2022
"""
function freeboard_to_thickness(
freeboard;
ice_density=917,
seawater_density6=1027,
ρsnow=350,
thickness_snow=0
)
thickness = freeboard * seawater_density / (seawater_density - ice_density) - #
snow_thickness * (seawater_thickness - rho_snow) / (rho_seawater - rho_ice);
return thickness
end
````
This is what it looks like in my terminal
---
### Author:  [@PeterSimon](https://discourse.julialang.org/u/PeterSimon)
#### Post date: [April 30, 2022, 2:43am UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/5 "2022-04-30T02:43:13Z")
A couple more suggestions:
- The semicolon ending the line `T = ...;` is unnecessary and unjulian.
- The line containing `return` is unnecessary, since in its absence the function will return the value of the final expression (`T` in this case). It’s not wrong, but after writing lots of Julia functions, most people don’t bother with a `return` statement if it is the final statement of the function.
---
### Author:  [@sbuercklin](https://discourse.julialang.org/u/sbuercklin)
#### Post date: [April 30, 2022, 3:41am UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/6 "2022-04-30T03:41:47Z")
It may be common practice, but the [Blue Style style guide](https://github.com/invenia/BlueStyle#method-definitions) says to always explicitly return a value for longform method definitions. If the method has no return value, it should still explicitly return `nothing`.
Implicit returns can be confusing to read, so I would agree that it’s best to always return _something_
---
### Author:  [@gustaphe](https://discourse.julialang.org/u/gustaphe)
#### Post date: [April 30, 2022, 3:48am UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/7 "2022-04-30T03:48:48Z")
> [@suavesito](#):
>
> maybe you want to use [Unitful.jl](https://github.com/PainterQubits/Unitful.jl) for the units. 😃
Note, OP, that this requires no change in the definition of the function. If you call this function with compatible unitful quantities, the result will be a unitful quantity too.
---
### Author:  [@FPGro](https://discourse.julialang.org/u/FPGro)
#### Post date: [April 30, 2022, 12:56pm UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/8 "2022-04-30T12:56:16Z")
But the default parameters would need to be uniful or you’d have to always explicitly give them, no?
---
### Author:  [@gustaphe](https://discourse.julialang.org/u/gustaphe)
#### Post date: [April 30, 2022, 4:14pm UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/9 "2022-04-30T16:14:37Z")
Correct. Since you cannot subtract a dimensionless number from a density, for instance.
---
### Author:  [@chadagreene](https://discourse.julialang.org/u/chadagreene)
#### Post date: [April 30, 2022, 8:19pm UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/10 "2022-04-30T20:19:00Z")
> [@PeterSimon](#):
>
> The semicolon ending the line `T = ...;` is unnecessary and unjulian.
Good to know. So when _should_ I be using a semicolon?
---
### Author:  [@jar1](https://discourse.julialang.org/u/jar1)
#### Post date: [April 30, 2022, 8:23pm UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/11 "2022-04-30T20:23:01Z")
I wouldn’t define `thickness` there, just return the value directly. We already know it’s a thickness from the name of the function and docstring.
---
### Author:  [@jar1](https://discourse.julialang.org/u/jar1)
#### Post date: [April 30, 2022, 8:24pm UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/12 "2022-04-30T20:24:54Z")
There might be a way to calculate the correct units for the default parameter values with Unitful.jl in terms of the positional argument’s type, too, but I don’t know the subject matter to do that.
---
### Author:  [@chadagreene](https://discourse.julialang.org/u/chadagreene)
#### Post date: [April 30, 2022, 8:25pm UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/13 "2022-04-30T20:25:08Z")
> [@suavesito](#):
>
> changed keyword arguments to be similar to base syntax of docstrings (you can see `?sort` )
I’m not sure I follow. Can you explain this point a little more?
Also, I see you removed the `Input` and `Output` sections of the docstring in your suggested rewrite. Why is that?
---
### Author:  [@PeterSimon](https://discourse.julialang.org/u/PeterSimon)
#### Post date: [April 30, 2022, 8:28pm UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/14 "2022-04-30T20:28:43Z")
Semicolons are typically only used to terminate lines in the REPL or in a notebook when you wish to suppress printout of the expression’s value. These printouts don’t occur for code executed from a file.
---
### Author:  [@gustaphe](https://discourse.julialang.org/u/gustaphe)
#### Post date: [April 30, 2022, 8:35pm UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/15 "2022-04-30T20:35:11Z")
Sure,
```julia
f(x; a=3) = a*x
f(x::Unitful.Quantity; a=3u"m") = a*x
```
---
### Author:  [@Olivier\_Merchiers](https://discourse.julialang.org/u/Olivier_Merchiers)
#### Post date: [April 30, 2022, 8:35pm UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/16 "2022-04-30T20:35:14Z")
The given suggestions are great so far.
I would like to make another one that would make your code slightly more julian.
If I inderstand your function correctly, what you compute is the thickness of the glacier from the freeboard?
Are there other things you can get glacier thicknesses from? If the answer is yes, I would do the following:
```julia
function glacier_thickness(f:: Freeboard)
F = f.F
rho_ice = f.rho_ice
rho_seawater =f.rho_seawater
rho_snow = f.rho_snow
T_snow = f.T_snow
return F*rho_seawater/(rho_seawater-rho_ice) - T_snow*(rho_seawater-rho_snow)/(rho_seawater-rho_ice);
end
```
Where Freeboard is a struct that contains all data:
```julia
Base.@kwdef struct Freeboard
F
rho_ice=917
rho_seawater=1027
rho_snow=350
T_snow=0
end
```
Maybe in the function you can use some macro to unpack the contents of the struct to avoid all the boilerplate.
Anyway, what this does is that now you can potentially make a second method (in julia parlance) of glacier\_thickness that acts on a different object like for a glacier on land for instance?
Like this you leverage what’s called multiple dispatch.
This is the way to make generic code since your function glacier\_thickness can be used on any object of relevance. You don’t need anylonger to define a function continentalfreeboard2thickness.
You could just do
```julia
function glacier_thickness(c::ContinentalFreeboard)
# I do stuff here
end
```
---
### Author:  [@jar1](https://discourse.julialang.org/u/jar1)
#### Post date: [April 30, 2022, 8:37pm UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/17 "2022-04-30T20:37:45Z")
I mean something more like
```julia
julia> f(x; y=3*unit(x)) = x + y
f (generic function with 1 method)
julia> f(10u"cm")
13 cm
```
---
### Author:  [@suavesito](https://discourse.julialang.org/u/suavesito)
#### Post date: [April 30, 2022, 9:06pm UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/18 "2022-04-30T21:06:31Z")
I removed output and input because I think is clear by the arguments what the input is, and by the description what the output is. This makes the docstrings more concrete and shorter.
By saying that I changed the styles of kwargs to match Base, I was meaning that the default values are specified directly in the function signature, this is how it is done in the docs for `sort`, as you may see in the REPL.
This also makes for the input section to be redundant, as the signature says the default values and the meaning of kwargs obvious. I know this is concise but not the clearer (for a beginner), but I do think this convention is better for the REPL help/docstring and something more elaborated to be in the documentation. 🙂
---
### Author:  [@CameronBieganek](https://discourse.julialang.org/u/CameronBieganek)
#### Post date: [May 1, 2022, 5:48am UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/19 "2022-05-01T05:48:23Z")
> [@Olivier\_Merchiers](#):
>
> Maybe in the function you can use some macro to unpack the contents of the struct to avoid all the boilerplate.
As of version 1.7, Julia has syntax for unpacking object properties:
```julia
julia> struct A
x
y
end
julia> a = A(1, 2)
A(1, 2)
julia> (; x, y) = a
A(1, 2)
julia> x
1
julia> y
2
```
(Also works for named tuples, or any object with a `getproperty` method.)
---
### Author:  [@gustaphe](https://discourse.julialang.org/u/gustaphe)
#### Post date: [May 1, 2022, 6:44am UTC](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280/20 "2022-05-01T06:44:53Z")
That only works if x and y necessarily has the same unit, and kind of removes the point of dimensional analysis. Say we have `f(x; y=3) = x + y`, and we want to change `y` to default to `3 m`, then we want `f(1u"s")` to fail, and `f(1u"cm")` to give `3.01 m`.
[Next page](https://discourse.julialang.org/t/please-give-feedback-on-my-function/80280.md?page=2)