neuronsimulator / nrn

NEURON Simulator
http://nrn.readthedocs.io
Other
376 stars 113 forks source link

DRY up `NPySecObj_*` accessors. #2921

Closed 1uc closed 1 week ago

1uc commented 2 weeks ago

The following piece of code:

static PyObject* NPySecObj_x3d(NPySecObj* self, PyObject* args) {
    Section* sec = self->sec_;
    CHECK_SEC_INVALID(sec);
    int n, i;
    if (!PyArg_ParseTuple(args, "i", &i)) {
        return NULL;
    }
    n = sec->npt3d - 1;
    if (i < 0 || i > n) {
        PyErr_SetString(PyExc_Exception, "Arg out of range\n");
        return NULL;
    }
    return PyFloat_FromDouble((double) sec->pt3d[i].x);
}

is duplicated for y, z, arc and d. This can easily be made more DRY.

mgeplf commented 2 weeks ago

How about something like:

static Pt3d* get_pt3d(NPySecObj* self, PyObject* args)
{
    Section* sec = self->sec_;
    CHECK_SEC_INVALID(sec);
    int n, i;
    if (!PyArg_ParseTuple(args, "i", &i)) {
        return nullptr;
    }
    n = sec->npt3d - 1;
    if (i < 0 || i > n) {
        PyErr_SetString(PyExc_Exception, "Arg out of range\n");
        return nullptr;
    }
    return &sec->pt3d[i];
}

static PyObject* NPySecObj_x3d(NPySecObj* self, PyObject* args) {
    Pt3d* pt3d = get_pt3d(self, args);
    if(pt3d == nullptr){
        return nullptr;
    }
    return PyFloat_FromDouble((double) pt3d->x);
}

It would probably also be better to give more helpful feedback than Arg out of range.

Alternatively, the

    Section* sec = self->sec_;
    CHECK_SEC_INVALID(sec);
    int n, i;
    if (!PyArg_ParseTuple(args, "i", &i)) {
        return nullptr;
    }

could be extracted, and then reused. And a macro to check that n lies within the range could be added. Preferences?