Difference between revisions of "Refactor C++ code to use ovl() when returning multiple values"

Introduction

Some Octave functions return more than one value. For example, the LU factorization can return L and U, but also P, Q, and R. In C++ these functions return an octave_value_list object which is a row vector of return values. The first element in the vector (element 0) contains the leftmost return argument. For return arguments L,U,P the indices would be 0,1,2.

However, unless the octave_value_list has been pre-declared to the correct size, growing the row vector by indexing into a non-existent position forces the array to grow. Thus, the straightforward and very clear coding strategy of

```retval(0) = L;
retval(1) = U;
retval(2) = P;
```

turns out to be an inefficient implementation. For this reason, current Octave code always writes this in reverse as

```retval(2) = P;
retval(1) = U;
retval(0) = L;
```

The first indexing operation resizes the row vector and is done only once, but the disadvantage is that this code is more difficult to match with the function's behavior.

As an alternative, there is the ovl() function where the name stands for Octave Value List. This function grows the vector just once and allows the arguments to be presented in their natural order. In this case, the example code could be written as

```retval = ovl (L, U, P);
```

The principal work for this sprint topic is to convert the C++ code to use the ovl() function where possible. In general, this means collapsing multiple assignment statements to a single assignment. Keeping in mind that line length should be limited to 80 characters, this may mean appropriately breaking the line and indenting the following lines. In some cases, the declaration of "octave_value_list retval" may no longer be necessary and a direct "return ovl (...)" will be possible. In that case, the code can be further simplified by eliminating the retval variable entirely from the function.

Example 1 : Remove List and retval declaration (colloc() from colloc.cc)

Before After
```retval(3) = q;
retval(2) = B;
retval(1) = A;
retval(0) = r;

return retval;
```
```return ovl (r, A, B, q);
```

Example 2 : ovl() extending over multiple lines

Before After
```if (nargout == 3)
{
retval(2) = eye.index (perm, idx_vector::colon);
retval(1) = U;
retval(0) = L.index (perm, idx_vector::colon) + eye;
}
else
{
retval(1) = U;
retval(0) = L + eye.index (perm, idx_vector::colon);
}
```
```if (nargout == 3)
{
retval = ovl (L.index (perm, idx_vector::colon) + eye,
U,
eye.index (perm, idx_vector::colon);
}
else
{
retval = ovl (L + eye.index (perm, idx_vector::colon), U);
}
```

Detailed Instructions

The list of files which contain a possible instance where ovl() could be used is shown in the Files section of this page. The actual instances, including line numbers, are shown in the Instances section. To avoid duplication, sign up for a particular file by editing the Files section of this wiki page and replacing '???' with your name. When you have edited a file you should verify that everything is okay by executing

```make all
./run-octave
test FILENAME
```

When that passes, let a Maintainer know so that we can check in the changes. Also, add the wiki tags

`<strike> ... </strike>`

to the Files section to cross the file off the list. In addition, increment the number of files that were fixed by +1.

Files

Start of Sprint

Total: 61

Fixed: 0

Owner File
??? corefcn/__dsearchn__.cc
??? corefcn/__ilu__.cc
??? corefcn/__qp__.cc
??? corefcn/balance.cc
??? corefcn/besselj.cc
??? corefcn/colloc.cc
??? corefcn/daspk.cc
??? corefcn/dasrt.cc
??? corefcn/dassl.cc
??? corefcn/data.cc
??? corefcn/debug.cc
??? corefcn/det.cc
??? corefcn/dirfns.cc
??? corefcn/eig.cc
??? corefcn/ellipj.cc
??? corefcn/file-io.cc
??? corefcn/filter.cc
??? corefcn/find.cc
??? corefcn/getgrent.cc
??? corefcn/getpwent.cc
??? corefcn/givens.cc
??? corefcn/graphics.cc
??? corefcn/help.cc
??? corefcn/hess.cc
??? corefcn/inv.cc
??? corefcn/lsode.cc
??? corefcn/lu.cc
??? corefcn/luinc.cc
??? corefcn/max.cc
??? corefcn/mgorth.cc
??? corefcn/oct-stream.cc
??? corefcn/ordschur.cc
??? corefcn/profiler.cc
??? corefcn/qz.cc
??? corefcn/regexp.cc
??? corefcn/schur.cc
??? corefcn/spparms.cc
??? corefcn/sqrtm.cc
??? corefcn/svd.cc
??? corefcn/symtab.cc
??? corefcn/syscalls.cc
??? corefcn/time.cc
??? corefcn/toplev.cc
??? corefcn/urlwrite.cc
??? dldfcn/__eigs__.cc
Andy dldfcn/__fltk_uigetfile__.cc
??? dldfcn/__glpk__.cc
??? dldfcn/__voronoi__.cc
??? dldfcn/amd.cc
??? dldfcn/ccolamd.cc
??? dldfcn/chol.cc
??? dldfcn/colamd.cc
??? dldfcn/convhulln.cc
??? dldfcn/dmperm.cc
??? dldfcn/qr.cc
??? dldfcn/symbfact.cc

Instances

Start of Sprint

Total: 410

Fixed: 0

```corefcn/__dsearchn__.cc:98
corefcn/__ilu__.cc:155
corefcn/__ilu__.cc:168
corefcn/__ilu__.cc:511
corefcn/__ilu__.cc:537
corefcn/__ilu__.cc:1000
corefcn/__ilu__.cc:1004
corefcn/__ilu__.cc:1011
corefcn/__ilu__.cc:1017
corefcn/__ilu__.cc:1052
corefcn/__ilu__.cc:1056
corefcn/__ilu__.cc:1063
corefcn/__ilu__.cc:1069
corefcn/__qp__.cc:518
corefcn/balance.cc:164
corefcn/balance.cc:169
corefcn/balance.cc:183
corefcn/balance.cc:188
corefcn/balance.cc:204
corefcn/balance.cc:209
corefcn/balance.cc:222
corefcn/balance.cc:227
corefcn/balance.cc:282
corefcn/balance.cc:285
corefcn/balance.cc:290
corefcn/balance.cc:307
corefcn/balance.cc:310
corefcn/balance.cc:315
corefcn/balance.cc:335
corefcn/balance.cc:338
corefcn/balance.cc:343
corefcn/balance.cc:360
corefcn/balance.cc:363
corefcn/balance.cc:368
corefcn/besselj.cc:132
corefcn/besselj.cc:147
corefcn/besselj.cc:173
corefcn/besselj.cc:191
corefcn/besselj.cc:206
corefcn/besselj.cc:229
corefcn/besselj.cc:243
corefcn/besselj.cc:269
corefcn/besselj.cc:287
corefcn/besselj.cc:301
corefcn/besselj.cc:534
corefcn/besselj.cc:551
corefcn/colloc.cc:98
corefcn/daspk.cc:448
corefcn/daspk.cc:453
corefcn/daspk.cc:458
corefcn/dasrt.cc:557
corefcn/dasrt.cc:562
corefcn/dasrt.cc:568
corefcn/dassl.cc:450
corefcn/dassl.cc:455
corefcn/dassl.cc:460
corefcn/data.cc:513
corefcn/data.cc:523
corefcn/data.cc:534
corefcn/data.cc:544
corefcn/data.cc:6393
corefcn/data.cc:6533
corefcn/debug.cc:1215
corefcn/det.cc:107
corefcn/det.cc:114
corefcn/det.cc:124
corefcn/det.cc:130
corefcn/det.cc:138
corefcn/det.cc:153
corefcn/det.cc:169
corefcn/det.cc:188
corefcn/det.cc:199
corefcn/det.cc:216
corefcn/det.cc:227
corefcn/dirfns.cc:185
corefcn/dirfns.cc:196
corefcn/dirfns.cc:200
corefcn/dirfns.cc:235
corefcn/dirfns.cc:262
corefcn/dirfns.cc:272
corefcn/dirfns.cc:307
corefcn/dirfns.cc:343
corefcn/dirfns.cc:369
corefcn/dirfns.cc:380
corefcn/dirfns.cc:404
corefcn/dirfns.cc:415
corefcn/dirfns.cc:440
corefcn/dirfns.cc:452
corefcn/dirfns.cc:454
corefcn/dirfns.cc:477
corefcn/dirfns.cc:488
corefcn/eig.cc:164
corefcn/eig.cc:215
corefcn/ellipj.cc:98
corefcn/ellipj.cc:114
corefcn/ellipj.cc:141
corefcn/ellipj.cc:175
corefcn/ellipj.cc:199
corefcn/ellipj.cc:235
corefcn/ellipj.cc:257
corefcn/ellipj.cc:292
corefcn/ellipj.cc:314
corefcn/file-io.cc:342
corefcn/file-io.cc:355
corefcn/file-io.cc:389
corefcn/file-io.cc:402
corefcn/file-io.cc:650
corefcn/file-io.cc:672
corefcn/file-io.cc:679
corefcn/file-io.cc:995
corefcn/file-io.cc:1019
corefcn/file-io.cc:1108
corefcn/file-io.cc:1125
corefcn/file-io.cc:1193
corefcn/file-io.cc:1220
corefcn/file-io.cc:1481
corefcn/file-io.cc:1514
corefcn/file-io.cc:1675
corefcn/file-io.cc:1868
corefcn/file-io.cc:1889
corefcn/file-io.cc:1926
corefcn/file-io.cc:1939
corefcn/file-io.cc:1958
corefcn/file-io.cc:1969
corefcn/filter.cc:448
corefcn/filter.cc:486
corefcn/filter.cc:527
corefcn/filter.cc:565
corefcn/find.cc:58
corefcn/find.cc:72
corefcn/find.cc:207
corefcn/find.cc:211
corefcn/find.cc:215
corefcn/find.cc:219
corefcn/find.cc:301
corefcn/find.cc:305
corefcn/find.cc:309
corefcn/find.cc:313
corefcn/getgrent.cc:84
corefcn/getgrent.cc:116
corefcn/getgrent.cc:148
corefcn/getgrent.cc:171
corefcn/getgrent.cc:194
corefcn/getpwent.cc:88
corefcn/getpwent.cc:120
corefcn/getpwent.cc:151
corefcn/getpwent.cc:174
corefcn/getpwent.cc:197
corefcn/givens.cc:99
corefcn/givens.cc:119
corefcn/givens.cc:142
corefcn/givens.cc:162
corefcn/graphics.cc:231
corefcn/graphics.cc:296
corefcn/graphics.cc:508
corefcn/graphics.cc:521
corefcn/graphics.cc:532
corefcn/graphics.cc:611
corefcn/graphics.cc:614
corefcn/graphics.cc:618
corefcn/graphics.cc:634
corefcn/graphics.cc:637
corefcn/graphics.cc:641
corefcn/graphics.cc:661
corefcn/graphics.cc:664
corefcn/graphics.cc:668
corefcn/graphics.cc:677
corefcn/graphics.cc:684
corefcn/graphics.cc:698
corefcn/graphics.cc:701
corefcn/graphics.cc:705
corefcn/graphics.cc:725
corefcn/graphics.cc:732
corefcn/graphics.cc:737
corefcn/graphics.cc:772
corefcn/graphics.cc:783
corefcn/graphics.cc:807
corefcn/graphics.cc:821
corefcn/graphics.cc:3862
corefcn/graphics.cc:3867
corefcn/graphics.cc:3872
corefcn/graphics.cc:3877
corefcn/graphics.cc:3882
corefcn/graphics.cc:3887
corefcn/graphics.cc:3892
corefcn/graphics.cc:3897
corefcn/graphics.cc:3902
corefcn/graphics.cc:3907
corefcn/graphics.cc:3912
corefcn/graphics.cc:3917
corefcn/graphics.cc:3922
corefcn/graphics.cc:3927
corefcn/graphics.cc:3932
corefcn/graphics.cc:3937
corefcn/graphics.cc:3942
corefcn/graphics.cc:3947
corefcn/graphics.cc:3952
corefcn/graphics.cc:3957
corefcn/graphics.cc:3962
corefcn/graphics.cc:3967
corefcn/graphics.cc:3972
corefcn/graphics.cc:3977
corefcn/graphics.cc:3982
corefcn/graphics.cc:5656
corefcn/graphics.cc:6758
corefcn/graphics.cc:6821
corefcn/help.cc:1104
corefcn/help.cc:1167
corefcn/hess.cc:102
corefcn/hess.cc:116
corefcn/hess.cc:133
corefcn/hess.cc:147
corefcn/inv.cc:195
corefcn/lsode.cc:430
corefcn/lu.cc:238
corefcn/lu.cc:240
corefcn/lu.cc:252
corefcn/lu.cc:256
corefcn/lu.cc:261
corefcn/lu.cc:264
corefcn/lu.cc:308
corefcn/lu.cc:310
corefcn/lu.cc:322
corefcn/lu.cc:326
corefcn/lu.cc:331
corefcn/lu.cc:334
corefcn/lu.cc:370
corefcn/lu.cc:379
corefcn/lu.cc:381
corefcn/lu.cc:405
corefcn/lu.cc:414
corefcn/lu.cc:416
corefcn/lu.cc:443
corefcn/lu.cc:452
corefcn/lu.cc:454
corefcn/lu.cc:478
corefcn/lu.cc:487
corefcn/lu.cc:489
corefcn/lu.cc:676
corefcn/lu.cc:694
corefcn/lu.cc:719
corefcn/lu.cc:737
corefcn/luinc.cc:215
corefcn/luinc.cc:217
corefcn/luinc.cc:235
corefcn/luinc.cc:240
corefcn/luinc.cc:292
corefcn/luinc.cc:294
corefcn/luinc.cc:312
corefcn/luinc.cc:317
corefcn/max.cc:61
corefcn/max.cc:99
corefcn/max.cc:265
corefcn/max.cc:271
corefcn/max.cc:278
corefcn/max.cc:891
corefcn/mgorth.cc:101
corefcn/mgorth.cc:110
corefcn/mgorth.cc:122
corefcn/mgorth.cc:131
corefcn/oct-stream.cc:4389
corefcn/ordschur.cc:145
corefcn/profiler.cc:442
corefcn/qz.cc:1143
corefcn/qz.cc:1147
corefcn/qz.cc:1151
corefcn/qz.cc:1161
corefcn/qz.cc:1166
corefcn/qz.cc:1168
corefcn/qz.cc:1175
corefcn/qz.cc:1177
corefcn/qz.cc:1182
corefcn/qz.cc:1184
corefcn/qz.cc:1198
corefcn/qz.cc:1210
corefcn/regexp.cc:370
corefcn/regexp.cc:390
corefcn/regexp.cc:397
corefcn/regexp.cc:410
corefcn/regexp.cc:416
corefcn/regexp.cc:451
corefcn/schur.cc:194
corefcn/schur.cc:210
corefcn/schur.cc:229
corefcn/schur.cc:245
corefcn/schur.cc:316
corefcn/schur.cc:326
corefcn/spparms.cc:119
corefcn/sqrtm.cc:231
corefcn/sqrtm.cc:250
corefcn/svd.cc:157
corefcn/svd.cc:163
corefcn/svd.cc:178
corefcn/svd.cc:184
corefcn/svd.cc:214
corefcn/svd.cc:234
corefcn/svd.cc:257
corefcn/svd.cc:277
corefcn/symtab.cc:1736
corefcn/syscalls.cc:98
corefcn/syscalls.cc:104
corefcn/syscalls.cc:125
corefcn/syscalls.cc:146
corefcn/syscalls.cc:176
corefcn/syscalls.cc:217
corefcn/syscalls.cc:272
corefcn/syscalls.cc:330
corefcn/syscalls.cc:469
corefcn/syscalls.cc:493
corefcn/syscalls.cc:525
corefcn/syscalls.cc:537
corefcn/syscalls.cc:551
corefcn/syscalls.cc:561
corefcn/syscalls.cc:689
corefcn/syscalls.cc:702
corefcn/syscalls.cc:786
corefcn/syscalls.cc:810
corefcn/syscalls.cc:846
corefcn/syscalls.cc:862
corefcn/syscalls.cc:874
corefcn/syscalls.cc:1224
corefcn/syscalls.cc:1247
corefcn/syscalls.cc:1260
corefcn/syscalls.cc:1318
corefcn/syscalls.cc:1340
corefcn/syscalls.cc:1555
corefcn/time.cc:493
corefcn/toplev.cc:963
corefcn/urlwrite.cc:392
corefcn/urlwrite.cc:398
corefcn/urlwrite.cc:495
dldfcn/__eigs__.cc:439
dldfcn/__eigs__.cc:473
dldfcn/__eigs__.cc:521
dldfcn/__eigs__.cc:567
dldfcn/__fltk_uigetfile__.cc:129 Andy: I think this is a case where using ovl makes no sense
dldfcn/__fltk_uigetfile__.cc:131 Andy: I think this is a case where using ovl makes no sense
dldfcn/__glpk__.cc:632
dldfcn/__voronoi__.cc:316
dldfcn/amd.cc:180
dldfcn/ccolamd.cc:322
dldfcn/ccolamd.cc:551
dldfcn/ccolamd.cc:570
dldfcn/chol.cc:201
dldfcn/chol.cc:203
dldfcn/chol.cc:208
dldfcn/chol.cc:226
dldfcn/chol.cc:228
dldfcn/chol.cc:233
dldfcn/chol.cc:258
dldfcn/chol.cc:275
dldfcn/chol.cc:297
dldfcn/chol.cc:314
dldfcn/chol.cc:679
dldfcn/chol.cc:860
dldfcn/colamd.cc:433
dldfcn/colamd.cc:623
dldfcn/colamd.cc:748
dldfcn/convhulln.cc:272
dldfcn/dmperm.cc:115
dldfcn/dmperm.cc:120
dldfcn/qr.cc:248
dldfcn/qr.cc:255
dldfcn/qr.cc:267
dldfcn/qr.cc:274
dldfcn/qr.cc:306
dldfcn/qr.cc:315
dldfcn/qr.cc:317
dldfcn/qr.cc:341
dldfcn/qr.cc:350
dldfcn/qr.cc:352
dldfcn/qr.cc:379
dldfcn/qr.cc:388
dldfcn/qr.cc:390
dldfcn/qr.cc:414
dldfcn/qr.cc:423
dldfcn/qr.cc:425
dldfcn/qr.cc:793
dldfcn/qr.cc:806
dldfcn/qr.cc:826
dldfcn/qr.cc:839
dldfcn/qr.cc:994
dldfcn/qr.cc:1011
dldfcn/qr.cc:1037
dldfcn/qr.cc:1053
dldfcn/qr.cc:1204
dldfcn/qr.cc:1219
dldfcn/qr.cc:1241
dldfcn/qr.cc:1256
dldfcn/qr.cc:1441
dldfcn/qr.cc:1452
dldfcn/qr.cc:1468
dldfcn/qr.cc:1479
dldfcn/symbfact.cc:312
dldfcn/symbfact.cc:320
dldfcn/symbfact.cc:327
dldfcn/symbfact.cc:337
```