Is this branching code I wrote a bit obfuscated?

Question

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?

Answer 1

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

Answer 2

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.]

Answer 3

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!

Answer 4

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}