# Is this branching code I wrote a bit obfuscated?

I've been away for a while and I forget if this sort of question is appropriate, but here goes. Recently I wrote some code (in expl3 but I hope it's clear enough) and in hindsight I wonder on its style. Some comments after the snippet (slightly adapted from unicode-math):

\cs_new:Nn \um_if_char_spec:nNNT
{
% case 1:
\seq_if_in:NnT \l_um_mclass_range_seq {#3} { \use_none_delimit_by_q_nil:w }

% case 2:
\seq_if_in:NnT \l_um_cmd_range_seq {#2} { \use_none_delimit_by_q_nil:w }

% case 3:
\seq_map_inline:Nn \l_um_char_range_seq
{
\um_int_if_slot_in_range:nnT {#1} {##1}
{ \seq_map_break:n { \use_none_delimit_by_q_nil:w } }
}

% this executes if no match was found:
\use_none:nnn
\q_nil
\use:n
{
\clist_put_right:Nx \l_um_char_num_range_clist { \int_eval:n {#1} }
#4
}
}


The idea is that three possibilities can cause a match and execute some additional "true" code. Checking for these matches can be time-consuming so any true occurrence should immediately jump to the end. This is done by \use_none_delimit_by_q_nil:w which skips ahead to the \q_nil token and ignores everything in its way. At which point it executes the "true" code.

This could normally be done with a set of nested conditionals like so:

 iftrue-(code)-else-(iftrue-(code)-else-(iftrue-(code))))


but I guess I didn't want to write out the (code) section several times — it seemed inelegant and error-prone. So what do you think? Is this code ugly? How would you write it?

-
I believe latex3 will be awarded the lifetime award for the most obfuscated programming language that is in real use. –  topskip Oct 25 '11 at 17:46
@Will: you didn't really explain what the code is meant to do (see comments below Yiannis' answer), and I had to gather some info from the unicode-math package. –  Bruno Le Floch Oct 26 '11 at 11:24
@Patrick: true. I teach my students to write C++ code such that all function names and variable are self understandable, so that one can understand the code by just reading it. I fail to understand expl3 code by just reading it. And thus I personally do not consider to learn it. –  Matthias Pospiech Oct 26 '11 at 11:49
@BrunoLeFloch — you're right, sorry for the unclear question. Serves me right for asking a question late at night. –  Will Robertson Nov 1 '11 at 0:19
@MatthiasPospiech — I don't mean to sound rude but it doesn't sound like you've done much reading of how expl3 is designed. The variables and functions are written to be understandable (they follow a strict naming structure), but you have to know some of the background of expl3 and to be fair this code snippet is completely out of context! I was only asking about code structure, not naming. In hindsight, I should have written a simpler example to do this. –  Will Robertson Nov 1 '11 at 0:30

I am not sure what would be the cleanest way to code this, but here is a proposal. As you say, the fourth argument is the "true code". This points me towards conditionals. However, you prefer avoiding to nest conditionals. For that, we need a way to jump over tokens until the end-marker, where we return either true or false.

\RequirePackage{expl3}
\ExplSyntaxOn
\cs_new_protected:Npn \um_if_char_spec:nNNT #1#2#3#4
{
\um_if_char_spec_aux:nNNT {#1} #2 #3
{
\clist_put_right:Nx \l_um_char_num_range_clist { \int_eval:n {#1} }
#4
}
}
\cs_new:Npn \um_break_true:  #1 \um_break_point: { \prg_return_true: }
\cs_new:Npn \um_break_false: #1 \um_break_point: { \prg_return_false: }
\prg_new_protected_conditional:Nnn \um_if_char_spec_aux:nNN { T }
{
% case 1:
\seq_if_in:NnT \l_um_mclass_range_seq {#3} { \um_break_true: }

% case 2:
\seq_if_in:NnT \l_um_cmd_range_seq {#2} { \um_break_true: }

% case 3:
\seq_map_inline:Nn \l_um_char_range_seq
{
\um_int_if_slot_in_range:nnT {#1} {##1}
{ \seq_map_break:n { \um_break_true: } }
}

% else:
\um_break_false:
\um_break_point:
}


Instead of having a custom "break_point" marker, we could simply use \q_nil as you did, and define

\cs_new:Npn \um_break_true: { \use_i_delimit_by_q_nil:nw { \prg_return_true: } }
\cs_new:Npn \um_break_false: { \use_i_delimit_by_q_nil:nw { \prg_return_false: } }


EDIT: after looking into the code of unicode-math, I am of the opinion that many of the sequences you work with could better be implemented as token lists. Namely, a sequence whose items are all single tokens, and for which the main operation you care about is \seq_if_in:NnTF, should be a token list: the search operation is then much faster, although mapping becomes a tad slower.

Given that the second and third arguments of \um_if_char_spec:nNNT are N-type arguments (see signature), I presume that \l_um_mclass_range_seq and \l_um_cmd_range_seq can be implemented as lists of single tokens. [I also note that elsewhere in the package you've been a little bit sloppy about N versus n arguments, feeding braced arguments to an N-type argument.]

-
This is a nice way to improve the syntax of the conditionals block. (Although for a once-off I'm still not sure if four-vs-one macro improves things...) thanks for your additional comments — some of this code is pretty old and needs revising; the n/N thing may well have been a search/replace typo. –  Will Robertson Nov 1 '11 at 0:42
@Will I think that it is not worth defining four macros in unicode-math, but perhaps something to add to the kernel. I'll open a github issue. –  Bruno Le Floch Nov 2 '11 at 15:19
Actually, there is already a similar issue that I didn't get time to resolve (converting all the various \..._break: to \prg_break. –  Bruno Le Floch Nov 2 '11 at 15:30

Yes​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​​

-
If it wasn't for the additional comment in the answer, this would be the shortest, upvoted answer to date. Beating this one by 11 characters. :) See the chat transcript by @PauloCereda. –  Werner Oct 25 '11 at 17:56
@Werner: Ah, but that's in a <sub><sup> so you have to quarter the character count, making it 12 characters in total (do punctuation and spaces count?). (More seriously, I thought it possible it might get mistaken for facetiousness if I didn't put that in.) –  Loop Space Oct 25 '11 at 18:07
@Andrew: now that the comments clarify what you meant, you can edit your answer to remove the comment. It could then end up with more votes than characters. –  Bruno Le Floch Oct 25 '11 at 21:35
Gosh, that was actually hard. An edited answer has to have at least 30 characters in the body. –  Loop Space Oct 26 '11 at 8:28
Thanks for the insight :-) –  Will Robertson Nov 1 '11 at 0:17

Yes.

Consider the problem of determining if letters a, b, c, are in either the Greek or the Latin or French alphabets. Using your suggested solution you will need to code 74 case statements.

Use lists, join the sequences and have only one test.

\documentclass{book}
\usepackage{lipsum,graphicx}
\begin{document}
\makeatletter
\def\IfIn#1#2{%
\def\check##1{%
\newif\ifin@
\in@{##1}{#2}
\ifin@ True Action
\else
False Action
\fi}
\@for \i:=#1\do{%
\expandafter\check\i%
}}
\IfIn{a,b,c,\delta,\gamma,}{a,b,c,d,\beta,\delta}
\makeatother
\end{document}


As to the notation, what is true in maths is true for code and I will quote Halmos.

Whenever it is possible to avoid the use of a complicated alphabetic apparatus, avoid it.

I personally think that the LaTeX3 Team is doing a great job, but I find it difficult to follow notation such as \um_if_char_spec_aux:nNNT. All I can say is that beautiful Pascal married TeX, but their children are ugly!

-
It's really just the Hungarian notation. LaTeX 3 wants to have types, but it can't. Actually, this is the best argument I've seen for Hungarian notation, since in a language with types, the compiler can help you out a bit. Here, only the notation gives a clue. –  Ryan Reich Oct 26 '11 at 1:23
@RyanReich Sure, but when names are sufficiently descriptive the additional type information can be redundant. Consider firstName with sFirstName or \seq_if_in with in where usage and type are obvious. Brains need grammar, read this aloud: \um_if_char_spec_aux and \if_char_spec_aux the latter is more understandable. –  Yiannis Lazarides Oct 26 '11 at 1:55
@Yiannis you should probably be using \in@{,#1,}{,a,b,...,\gamma,} if the items can be longer than one token, or use \in@{#1}{ab...\gamma} if there is only ever going to be one token per item. In fact, \seq_if_in:NnT does something completely different from \in@. It expects a sequence, which is not a general list of tokens: it is closer to Knuth's \\{...}\\{...} lists, and that allows arbitrary items. Good or bad, it depends. –  Bruno Le Floch Oct 26 '11 at 2:56
Sorry for a double comment, I realized that when editing my answer. What Will is doing here is testing if #2 is in one sequence, or if #3 is in another sequence, so I don't see how relevant your example is here. Analog would be "is #1 in the Greek alphabet or #2 in the Latin alphabet". The LaTeX3 code for what you wrote is \tl_if_in:nnTF {abc\alpha\beta\gamma}{#1}{True action}{False action}. Not much longer. –  Bruno Le Floch Oct 26 '11 at 3:18
@Yiannis I don't see how "Your code is obfuscated, here's some code to do something else." is a useful answer. Besides, your first statement is wrong. Using Will's approach to your problem is three statements, \seq_if_in:Nn \l_alphabets_seq { a }, \seq_if_in:Nn \l_alphabets_seq { b }, and \seq_if_in:Nn \l_alphabets_seq { c }, not 74. –  Bruno Le Floch Oct 26 '11 at 13:07

If it's opinion you want (in answer to your first question), here's one...

From a programming point of view, I think there is little difference in coding it using jump rather than nesting ifs. The executing times should be comparable. However, since I'm unfamiliar with expl3 code, I am not sure how \use_none_delimit_by_q_nil:w works. If it has to scan tokens until it finds \q_nil then it probably is not a true "jump" and might therefore be slower than to use a macro definition for code (which is more "jump"-ish in nature). If code is compact, then scanning over its should not be a big deal.

I guess my general opinion would be summarized based on the size/span of code. If code is large, then put it in a macro do condense the look and improve readability of your code. If code is small, then included it as-is.

That's probably about 1.5c's worth of input

-
I can confirm that using jump or nested conditionals is the same performance-wise here. \use_none_delimit_by_q_nil:w is defined as \long macro:#1\q_nil ->, so that's just as quick as any other macro parameter grabbing. –  Bruno Le Floch Oct 25 '11 at 17:44