Coding style
Comment your code
My father brought be up with the principle that you should be able to reconstruct your code from your comments. Maybe that level of commenting isn't necessary, especially if you use well-chosen names, but at least comment your API functions with what they do, what the arguments are, and what the results are.
Adopt a naming convention
You make use of constants, variables, functions, and operators, but without any discernible naming convention other than namespaces being uppercased and everything else lowercased. Furthermore, multi-word names and abbreviations in names are all run together. This makes it hard for the human reader to parse your code and remember calling conventions. Consider adopting or adapting a naming convention and separating words and abbreviations with under_scores or camelCase. As an example, take my personal naming conventions.
Explain your names
Many of your names are very short. Consider adding a comment when such names are first used, explaining their mnemonic, as this will help the reader of your code.
Avoid excessive use of ⍨
to swap arguments
I'm a big proponent of using ⍨
to avoid parentheses, however, if using ⍨
leads to the left argument needing a parenthesis then there is nothing to be gained. For example, (r←255÷⍺-1)÷⍨⍵
can become ⍵÷r←255÷⍺-1
and (¯4×1+wl)÷⍨i-+/(wl*2 1 0)×1 4 3×⍺
can be (i-+/(wl*2 1 0)×1 4 3×⍺)÷¯4×1+wl
.
Use ⍨
to simplify code
Whether to use ⍨
in simple cases or not, is a matter of style. I personally do it, and I see you have too e.g. with ⍺ ##.tiling⍨⍴⍵
. However, I'd be consistent then and also do it with e.g. (x,8×⌈8÷⍨y)⍴data
as data⍴⍨x,8×⌈8÷⍨y
and (⍴⍵)##.tiling ⍺
as ⍺ ##.tiling⍨ ⍴⍵
and (⍴⍵)⍴1
as 1⍴⍨⍴⍵
although this latter example could also be written as 1⍨¨⍵
, utilising the "Constant" meaning of ⍨
.
Remove unnecessary parentheses
Besides for governing order of execution and binding, parentheses can clarify structure in the code. APL's very simple precedence rules means that there's no need for parentheses "just to be sure". Even in the absence of a naming convention, primitives have a clear syntactic role, and thus things like (##.quant⍥##.linear)¨
can be ##.quant⍥##.linear¨
. Alternatively, you could "factor out" the dotting into the parent space as ##.(quant⍥linear¨)
or ##.(quant⍥linear)¨
.
There's no need for any parenthesis in ⎕UCS(⍕⌽⍴img)
and ⎕UCS(⍕x y←⌽1↓⍴img)
.
Neither is there in {+/,⍵}⌺(⍺ ⍺)
as stranding binds stronger than almost anything else.
Avoid old-school use of →
Your uses of →
can and should be replaced by proper control structures for clarity and to avoid coding errors. E.g. loop:
… →(l≠v)/loop
becomes :Repeat
… :Until l=v
and
→type/pbm pgm ppm
pbm:
data←,⍉(8⍴2)⊤256|(⍳∘10↓⊢)data
img←y(↑⍤1)(x,8×⌈8÷⍨y)⍴data
→0
pgm:
img←x y⍴256|(⍳∘10↓⊢)⍣2⊢data
→0
ppm:
img←(1⌽⍳3)⍉x y 3⍴256|(⍳∘10↓⊢)⍣2⊢data
→0
becomes
:Select type⍳1
:Case 1 ⍝ pbm
data←,⍉(8⍴2)⊤256|(⍳∘10↓⊢)data
img←y(↑⍤1)(x,8×⌈8÷⍨y)⍴data
:Case 2 ⍝ pgm
img←x y⍴256|(⍳∘10↓⊢)⍣2⊢data
:Case 3 ⍝ ppm
img←(1⌽⍳3)⍉x y 3⍴256|(⍳∘10↓⊢)⍣2⊢data
:EndSelect
Similarly in writepnm
.
Be consistent in ⎕IO
usage
Some of your functions use ⎕IO
explicitly to be ⎕IO
-independent, some localise ⎕IO←0
and some ⎕IO←1
. Your code would be easier to follow if you settled on a specific value and set it once in the outermost namespace.
Alternatively, use one main value and set the other locally when needed. With this usage, the main value is usually 1
and the local 0
.
Avoid inline anonymous multi-line functions
The inner multi-line dfn in diffuse
is being used in exactly the same manner as bforg
in gblur
, yet isn't named and called separately:
⊃{
b←0.5≤a←in×m=⍺
err←a-b
c←err÷cvol m>⍺
in+←cvol c
b∨⍵}/(⌽⍳≢,⍺),0
Using inline dfns like this is not only hard to read, it is also very confusing to trace through and debug because the flow is in the following order:
7{
2
3
4
5
6 }/1
Give the function a proper name and use that.
Parenthesise multiple assignment
Dyalog recommends that the names (…) are enclosed in parentheses to reduce potential ambiguity in assignment statements. [source]
Thus, l rad←
should be written as (l rad)←
and x y←
should be (x y)←
Reverse order of variable names instead of reversing the data
x y←⌽
can be simplified to y x←
.
Avoid unnecessary trains
Sometimes a plain explicit expression does the trick: (⌊0.5+⊢)
can simply be ⌊0.5+
which will also run faster.
Functions should return a result
Though no result is needed from writepnm
, it is still good practice to return a result. If you do not return a result, it is very awkward to use the function from inside dfns, or inline in expressions including trains. To prevent cluttering when used in an APL session, you can make the result shy by putting the result in braces in the function header: {result}←img writepnm(spec file);hdr;dta;x;y;tie
. A possible sensible result could be the number of bytes written or 1
to indicate that all went well.
Use ⎕NAPPEND
rather than ⎕ARBOUT
⎕NAPPEND
is the normal way to write to binary files.
Unnecessary reshaping
As far as I can tell, the reshaping in ,y(3×x)⍴
is a no-op since the data is ravelled immediately afterwards.
Avoid reusing variable names for unrelated values
In the single expression s←⍴g←s⌿(s←0≠+/g)/g←⌊0.5+(⊢××/∘⍴)gauss r
the variable name s
is used for two unrelated values. This is bound to confuse the casual reader.
Make sure code is restartable
The code s←⍴g←s⌿(s←0≠+/g)/g←⌊0.5+(⊢××/∘⍴)gauss r
changes the value of g
and s
twice. This means that if something goes wrong in this line, and you have to re-evaluate it, g
and/or s
may already have gone through their first transformation, and you'll have to back up until their initial assignment. Instead, consider breaking the expression into three lines, which also becomes much easier to read:
g←⌊0.5+(⊢××/∘⍴)gauss r
s←0≠+/g
s←⍴g←s⌿s/g
In fact, the refactoring might inspire you to make the filtering of g
in-place:
g←⌊0.5+(⊢××/∘⍴)gauss r
s←0≠+/g
g/⍨←s ⋄ g⌿⍨←s
s←⍴g
Use ⍣
for more elegant code
imax←,⍳(⌈/⌈/)
can be written as imax←,⍳⌈/⍣2
which hints at the rank too.
Use ∘
to simplify tacit functions
Thus ⊢-(~2∘|)
can be written as ⊢-∘~2∘|
.
File organization
You've organised all code into two scripted namespaces, which means that changing any code affects the source file for a lot of other code. Also, the casing of your file names does not match the casing of their contents.
Instead, use Link and make every namespace and sub-namespace represented by a folder and subfolder, of matching name, in your repo. You can either choose to keep leaf namespaces scripted, or even break those up with one function/operator in each file.
If you unscript everything your only source files will be .apln for functions and .aplo for operators. However, you will have to wrap tacit functions in tradfns covers, e.g.:
tiling←tiling
tiling←⊣⍴⊢/⍤⊣⍴⍤1⊢
Either scheme will allow you to edit and track changes on a much more granular basis, will allow you to use file handling to move items around, and will allow you to edit multiple items simultaneously.
Error handling and defensive programming
Raise proper errors rather than returning messages
Your PNM file handling functions return character vector messages, which are non-conforming results, rather than signalling proper errors when something goes wrong. This would make for rather awkward usage, as one has to check the return value rather than simply trapping errors. Read up on error trapping with Dyalog APL (note the list of external links at the bottom).
Don't forget to untie the file if an error happens!
Assert valid input
readpnm
goes right ahead and attempts a ⎕MAP
without checking that the input is even a character vector or that the file exists. If fed an invalid array or even a filename for a file that doesn't exist, the user will see readpnm
suspend into the tracer. Consider wrapping the code body in:
:If 1≥|≡file ⍝ vector
:AndIf 0 2∊⍨10|⎕DR file ⍝ character
:AndIf ⎕NEXISTS file
⋮
:Else
⎕SIGNAL⊂('EN' 11)('Message' 'Invalid file name')
:EndIf
Similar tests can be done for other API functions.
Avoid unnecessary option setting
⎕NCREATE
will error if you try to create a file that already exists, so there's no need to use variant in (⎕NCREATE⍠'IfExists' 'Error')
.
Avoid dangerous usage of ⍎
The code x y←⌽⍎
blindly executes part of the file contents, which should generally be avoided. Consider either filtering the file contents or using the safe ⎕VFI
: y x←2↑⊃⊢⍤//⎕VFI
(The 2↑
here is to ensure we get exactly two dimensions and thus avoid a length error upon assignment.)
Performance improvements
Use built-in type conversion rather than implementing it
As far as I can tell, ,⍉(8⍴2)⊤256|
converts signed 1-byte integers to signed 1-byte integers, and then further to individual bits. You can probably speed things up by only modifying the internal type (which doesn't actually change the data bits) rather than doing the computation: 11⎕DR⊃0 83⎕DR
. Note the 0 83⎕DR
which ensures the numbers are interpreted as unsigned integers even if they have been squeezed to Boolean (when all numbers are zeros and ones).
Use in-place changes to avoid memory copying
When reading a PBM file, (x,8×⌈8÷⍨y)⍴data
reshapes data
for creating img
but data
is never used again. Since the interpreter doesn't know that data
won't be used, it has to keep its value available, and thus the reshaping requires copying the entire data in memory. If instead you do a modified assignment to perform the reshaping, it can be done in-place: data⍴⍨←x,8×⌈8÷⍨y
Similarly for PGM and PPM files, you might be able to avoid a memory copy by computing the amount of data to drop and then dropping it in-place: data↓⍨←2⍳⍨+\10=data
– however, be aware that this traverses the data in its entirety to finding all the 10
before it can identify the second 10
, so it might actually suffer in performance. Morale: Run some speed tests.
Avoid grade for finding the position of the smallest element
quant
uses ⊃∘⍋⍤1
to find the position of the smallest element in each row. Depending on your Dyalog version, you may find (⊢⍳⌊/)⍤1
to be significantly faster.
Consider computing constants once
The constant g←1+∘.+⍨0 1 0
will be recomputed every time diffuse
is used. Consider defining it once in the namespace, or at least simplifying its definition to g←3 3⍴1 2 1 2 3 2 1 2 1
.