[Info-vax] BASIC (and Horizon)

[Dan Cross]

In article <upeokq$1o97f$1 at dont-email.me>,
Arne Vajhøj  <arne at vajhoej.dk> wrote:
>On 1/31/2024 6:21 PM, Lawrence D'Oliveiro wrote:
>> On Wed, 31 Jan 2024 17:08:44 -0500, Arne Vajhøj wrote:
>>> On 1/31/2024 4:28 PM, Lawrence D'Oliveiro wrote:
>>[snip]
>> Nesting.
>
>Goto work fine with nested loops. In fact it is one of the cases
>where it really makes sense.

I think the troll was referring to nested control structures,
with nested resource handling.

>>           Try to scale up to something like
>> 
>> static PyObject * discipline_makedict
>>    (
>>      PyObject * self,
>>      PyObject * args
>>    )
>>    {
>>      PyObject * result = NULL;
>>      PyObject * tempresult = NULL;
>>      br_PyObject * items;
>>      const br_char * msg = NULL;
>>      do /*once*/
>>        {
>>          const bool parsed_ok = PyArg_ParseTuple(args, "Os", &items, &msg);
>>          if (not parsed_ok)
>>              break;
>>          fprintf(stdout, "makedict says: “%s”\n", msg);
>>          if (not PyTuple_Check(items))
>>            {
>>              PyErr_SetString(PyExc_TypeError, "expecting a tuple");
>>              break;
>>            } /*if*/
>>          const ssize_t nr_items = PyTuple_Size(items);
>>          if (PyErr_Occurred())
>>              break;
>>          tempresult = PyDict_New();
>>          if (tempresult == NULL)
>>              break;
>>          for (ssize_t i = 0;;)
>>            {
>>              if (i == nr_items)
>>                  break;
>>              br_PyObject * const item = PyTuple_GetItem(items, i);
>>              if (item == NULL)
>>                  break;
>>              if (not PyTuple_Check(item) or PyTuple_Size(item) != 2)
>>                {
>>                  PyErr_SetString(PyExc_TypeError, "expecting a 2-tuple");
>>                  break;
>>                } /*if*/
>>              br_PyObject * const first = PyTuple_GetItem(item, 0);
>>              if (first == NULL)
>>                  break;
>>              br_PyObject * const second = PyTuple_GetItem(item, 1);
>>              if (second == NULL)
>>                  break;
>>              if (first == (PyObject *)&ExceptMe_type or second == (PyObject *)&ExceptMe_type)
>>                {
>>                  PyErr_SetString(PyExc_ValueError, "ExceptMe object found");
>>                  break;
>>                } /*if*/
>>              if (PyDict_SetItem(tempresult, first, second) < 0)
>>                  break;
>>              ++i;
>>            } /*for*/
>>          if (PyErr_Occurred())
>>              break;
>>        /* all done */
>>          result = tempresult;
>>          tempresult = NULL; /* so I don’t dispose of it yet */
>>        }
>>      while (false);
>>      Py_XDECREF(tempresult);
>>      return
>>          result;
>>    } /*discipline_makedict*/
>
>That code would look a lot cleaner if the do while(false) loop got
>removed and the relevant breaks got replaced by goto's.

I almost agree.  That could is borderline unreadable, but if one
detangles it from the weird web of style its obfuscated by, one
realizes it can be written in a far more straight-forward manner
without either `goto` or the weird do/while style.  Near as I
can tell, the following rough cut is equivalent to the original:

static PyObject *
discipline_makedict(PyObject *self, PyObject *args)
{
        br_PyObject *items;
        const br_char *msg = NULL;

        const bool parsed_ok = PyArg_ParseTuple(args, "Os", &items, &msg);
        if (!parsed_ok)
                return NULL;

        printf("makedict says: \xe2\x80\x9c%s\xe2\x80\x9d\n", msg);
        if (!PyTuple_Check(items)) {
                PyErr_SetString(PyExc_TypeError, "expecting a tuple");
                return NULL;
        }
        const ssize_t nitems = PyTuple_Size(items);
        if (PyErr_Occurred())
                return NULL;

        PyObject *result = PyDict_New();
        if (result == NULL)
                return NULL;

        for (ssize_t i = 0; i < nitems; ++i) {
                br_PyObject *const item = PyTuple_GetItem(items, i);
                if (item == NULL)
                        break;
                if (!PyTuple_Check(item) || PyTuple_Size(item) != 2) {
                        PyErr_SetString(PyExc_TypeError, "expecting a 2-tuple");
                        break;
                }
                br_PyObject *const first = PyTuple_GetItem(item, 0);
                if (first == NULL)
                        break;
                br_PyObject *const second = PyTuple_GetItem(item, 1);
                if (second == NULL)
                        break;
                if (first == (PyObject *)&ExceptMe_type || second == (PyObject *)&ExceptMe_type) {
                        PyErr_SetString(PyExc_ValueError, "ExceptMe object found");
                        break;
                }
                if (PyDict_SetItem(result, first, second) < 0)
                        break;
        }

        if (PyErr_Occurred()) {
                Py_XDECREF(result);
                return NULL;
        }

        return result;
}

I would argue this is more idiomatic, shorter, simpler, easier
to both read and to reason about, and uses fewer variables.

I couldn't really figure out why the `printf` was in there, so I
left it in; it looks like debugging code though.  I didn't test
it, however, since I don't really care.

>A well named goto label is much more informative than a plain break.

Agreed.  In this case, the breaks are sufficiently contained
once the rest of the cruft is cut away that I don't think a goto
is any better.

>> More details here
>> <https://gitlab.com/ldo/a_structured_discipline_of_programming/>.
>
>Truly bad design.
>
>All the code examples would look better with the do while(false) loops
>removed and appropriate goto's.

Agreed.  That code is straight garbage, and should not make it
past any serious review.

	- Dan C.