• Leo (unregistered)

    It's certainly a WTF, and no, it's not clear code.

  • 1 (unregistered)

    [[Frist, 0, 0], [0, Frist, 0], [0, 0, Frist]]

  • JO (unregistered)

    I don't understand the specification. An example where n=4, but n is still used in the example?

  • Someone (unregistered)

    Hmmm.. the "case 1" and "case n" parts could go before or after the loop, and the loop would be from 2 to n-1. So no, in this case (pun wasn't intended, but is now) it's not necessary, but could be otherwise.

  • I'm not a robot (unregistered)

    The point of the for/case antipattern is that it does something completely different on each iteration. It isn't simply "any case statement inside a loop", and anyone who thinks that's inherently bad without further context needs their head examined.

  • Warren (unregistered) in reply to I'm not a robot

    As above, this is surely not a WTF.

    The "worse" challenge is too easy. Anything can be implemented badly given a choice of language and limited requirements!

    I suspect TRWTF is the reason for these requirements - whatever's using the output should probably have the logic in it - but we'll never know.

  • Mika (unregistered)

    Okay, now how to do it better (python with numpy):

    import numpy as np

    n = 4

    e_1 = np.zeros(n) e_1[0] = 1

    e_n = np.zeros(n) e_n[n-1] = 1

    n_1 = np.identity(n)[:, 1:]

    n_n = np.identity(n)[:, :-1]

  • Mika (unregistered)

    stupid formatting, now with line breaks:

    import numpy as np

    n = 4

    e_1 = np.zeros(n)

    e_1[0] = 1

    e_n = np.zeros(n)

    e_n[n-1] = 1

    n_1 = np.identity(n)[:, 1:]

    n_n = np.identity(n)[:, :-1]

  • Mika (unregistered)

    I don't know the exact syntax, but you can simply unroll the for-case-loop and be happy:

    instead of:

    For i = 1 To n

    Select Case i

    Case 1:                                 ' first row of e_1 and n_n have values

        e_1(i, 1) = 1#

        n_n(i, i) = 1#

    Case n:                                 ' last row of e_n and n_1 have values

        e_n(i, 1) = 1#

        n_1(i, i - 1) = 1#

    Case Else                               ' middle rows of n_1 and n_n have values

        n_1(i, i - 1) = 1#

        n_n(i, i) = 1#

    End Select

Next i

    you can do:

       ' first row of e_1 and n_n have values

   e_1(i, 1) = 1#

   n_n(i, i) = 1#

   ' middle rows of n_1 and n_n have values

   For i = 2 to n-1:

        n_1(i, i - 1) = 1#

        n_n(i, i) = 1#

   Next i

   ' last row of e_n and n_1 have values

   e_n(n, 1) = 1#

   n_1(n, n - 1) = 1#

    there, better. The same thing. People are just weirdly afraid of for-loops counting from 2 to n-1 instead of from 1 to n. There is nothing wrong with counting from 2 to n-1.

  • Mika (unregistered)

    You can actually even replace the somewhat cumbersome n_1 = np.identity(n)[:, 1:] n_n = np.identity(n)[:, :-1] with n_1 = np.eye(n, n-1, -1) n_n = np.eye(n, n-1)

    in python with numpy. If you do matrix math, just use a proper library with batteries included.

  • Dan Bugglin (google)

    You only need two lines of code with no for loop to take care of the two smaller arrays. From there the problem is simplified to a single small for loop with no switch/select.

    Public Sub DefineProjectionArrays(ByVal n As Long, ByRef e_1 As Variant, ByRef e_n As Variant, ByRef n_1 As Variant, ByRef n_n As Variant) Dim i As Long, j As Long

    ' Generate 4 Arrays of the form (example n=4)
'       | 1 |         | 0 0 0 |
' e_1 = | 0 |   n_1 = | 1 0 0 |
'       | 0 |         | 0 1 0 |
'       | 0 |         | 0 0 1 |
'
'       | 0 |         | 1 0 0 |
' e_n = | 0 |   n_n = | 0 1 0 |
'       | 0 |         | 0 0 1 |
'       | 1 |         | 0 0 0 |

' Fill arrays with zeros
e_1 = NewArray(n, 1, 0#): e_n = NewArray(n, 1, 0#)
n_1 = NewArray(n, n - 1, 0#): n_n = NewArray(n, n - 1, 0#)

e_1(1, 1) = 1#
e_n(n, 1) = 1#

For i = 1 To n - 1
    n_1(i + 1, i) = 1#
    n_n(i, i) = 1#
Next i

    End Sub

  • Someone (unregistered) in reply to Mika

    Uh, that 2 to n-1 idea is exactly what I wrote.

  • Brian Boorman (google)

    TRWTF is that if your going to be using matrix math, you should be using a tool designed specifically for matrix math. Like Matlab. Sure, you can do a lot of Engineering with Excel, but use the proper tool for the job please.

    Addendum 2017-05-10 08:36: *you're. damnit.

  • Commenter_001 (unregistered)

    def f(n): return (lambda x,y: ([1]+x,x+[1],[[0]*n]+y,y+[[0]n]))([0](n-1),[[int(x==y) for x in range(n)] for y in range(n)]) much clearer

  • Commenter_001 (unregistered)

    addendum: the comment formatter made *'s into emphasis /inside/ a code tag. there should be a manual for commenting here ...

  • kktkkr (unregistered)

    Yes, there should. Maybe markdown syntax will do the trick? Let's try...

    Everything Is A Python Oneliner: e_1,n_1,e_n,n_n=[map(lambda x:map(int,x[slice(*s)]),[str(10**n+10**i)[1:] for i in reversed(range(n))]) for s in ([-1,None],[1,None],[None,1],[None,-1])]

  • Sam (unregistered)

    MATLAB:

    function [ e_n, n_n, e_1, n_1 ] = defineprojectionarrays( n )

e_1=[1; zeros(n-1,1)];
e_n=[zeros(n,1); 1];

n_1=[zeros(1,n-1); eye(n-1)];
n_n=[eye(n-1); zeros(1,n-1)];

end
  • Sam (unregistered)

    Another Matlab way is to n_n=flipud(fliplr(n_1))

  • Bob (unregistered)

    If you know you want 4 arrays filled with the specified values, why bother generating it?

    Why not just statically declare them?

    WTF?

  • Primary Key (unregistered) in reply to Bob

    Like. Same idea I had.

  • RTFM (unregistered) in reply to Bob

    Because the sizes are dynamic. n is passed in as a parameter.

  • Not Try (unregistered) in reply to Bob

    Agree, this is trying to solve a non-problem. Just type out the definition fucking try hards. Probably has fastest computation speed too.

  • (nodebb)

    What I see as the quintessential for-case antipattern is:

    • The select case is the entire body of the for loop
    • The iteration variable is only referenced in select case i
    • (optional) case else is not used.

    In this case, the fact case else contains a significant amount of code (compared to the rest of the block) and uses the iteration variable means it's not for-case.

  • Bob (unregistered) in reply to RTFM

    Ah...missed that...thanks :)

    Quite frankly, I'm not recoiling in horror from the original code. Seems legit.

  • Josh (unregistered) in reply to Dan Bugglin

    That was certainly my first thought, Dan... why the hell do you need a loop to deal with the one value in each of the single-dimension arrays?

    Your code is what I would have written, and seems more intuitive to me as a result.

    I did wonder why the smaller arrays couldn't just point to the appropriate column (or row) of the square arrays, though...

  • Bob (unregistered) in reply to Josh

    Watch out for overshooting the end of that array ;)

  • El Duderino (unregistered)

    I kinda prefer a threesome of arrays. A little easier to populate...

  • Sole Purpose of VIsit (unregistered)

    I'm with the guys who point out that the real problem with the solution isn't the "anti-pattern:" it's a failure to understand the abstractions involved. (Which can often lead to an anti-pattern, I suppose.)

    The first thing I thought when I saw the "documenting comment" was, oh boy, I wonder what the requirement is. I mean, there must be some reason to need these data structures. And there's every reason to believe that they are part of a bigger requirement, which may not yet be obvious, but will no doubt develop in time.

    The second thing I thought was "Oh, look, it's sort of an identity matrix, except it isn't quite." This leads to the numpy solution given above, because the clearest and most maintainable solution is to create an identity matrix of the appropriate dimension and then to use matrix manipulation to produce the desired result. That way you are operating on actual matrixes, not on some arbitrary loop of ones and zeros.

    The slight problem with using either numpy or Matlab is that, well, this is a VB.Net environment, so those don't really help that much. However, if you google/stack-overflow for .NET matrix libraries, you should be able to find a reasonable fit. (I found several.)

  • John (unregistered)

    I desperately want to show some APL but I've not used it for 30 years.

  • Sam (unregistered) in reply to Sole Purpose of VIsit

    Taken your comment to the logical conclusion then is not to create the matrix directly but in say, MATLAB, build the matrix with maketform('projective',...) or fitgeotrans or whatever, in which case the matrix is treated as an opaque operator. I find that a lot of programmers, versus engineers and scientists, have a knee-jerk reaction against this level of abstraction, in part because it requires a specialized environment that has as many of the tools you need as possible, Matlab or SciPy. But also because of the large level of disconnect between what they perceive as programming. (For example, Matlab matrices are not arrays, they're complex copy-on-write structures that are completely opaque but make a lot of operations, like transposing, O(1)). Ultimately, though it is the right tool for the majority of engineering-heavy small-scale problems.

  • Sole Purpose of VIsit (unregistered) in reply to Sam

    I'd put it more simply in this case. 99% of programmers are not comfortable with matrix arithmetic. (I'm not particularly comfortable, for example.)

    But my basic point is, if you are working in a domain where matrix transformations apply, then you are doing everybody else a disservice by not using matrix arithmetic in some way.

    I mean, let's assume the product or project behind this thing uses eigenvectors or linear algorithms or any number of other things that are based upon matrix transforms. And let's assume that there are five or ten VB.Net programmers out there, each charged with a sub-task involving the implementation of one small part of what is essentially one, properly abstracted to the correct level, domain.

    It isn't actually important that this one is an anti-pattern that obfuscates the abstractions inherent in the domain. It's important that there are five or ten other VB.Net programmers (in this case, the language is irrelevant) who are going to come up with five or ten completely separate and incompatible anti-patterns.

    This is why I despise "clean code" freaks. Don't tackle the problem at the lowest level possible. Tackle it at the appropriate level, which is probably design and possible even architecture.

    Rant over.

  • Sole Purpose of VIsit (unregistered)

    For "linear algorithms" please read "linear algebra." Sorry 'bout that.

  • phate (unregistered)

    You just need to create a nxn identity matrix and slice it properly... like, in python (I don't know VBA, but this shouldn't be that hard...):

    tmp = numpy.identity(n)

    e_1 = tmp[:,0] e_n = tmp[:,n-1] n_1 = tmp[:,1:n-1] n_n = tmp[:,0:n-2]

  • Sean Patrick Santos (github)

    I'm not sure that this counts an example of the "for-case" antipattern. That antipattern usually implies that the loop has a fixed (small) number of iterations, and it does not save any effort at all, i.e. that you could fully unroll the loop and have the code be no more verbose than before. In other words, my understanding of the "for-case" antipattern is that it's only applicable for problems where a loop is unnecessary in the first place. That said, I can't remember the last time where a "for-case" was useful for anything other than processing some stream of data coming in from outside the process (e.g. event handlers, simple command line option parsing).

    Anyway, I'm not really sure what the reason is for interleaving the definitions of the different variables in this example. I've hardly used any form of Visual Basic (and when I did it was over a decade ago), but aside from possible syntax errors, my "better" answer would have been something like the below. (The comments assume that these arrays are matrices for some linear algebra problem, which seems like a reasonable guess based on the function name, and the pattern mentioned in the comments.)

    ' e_1 is a vector of size n, consisting of a 1 followed by n-1 0s.
e_1 = NewArray(n, 1, 0#)
e_1(1, 1) = 1#

' e_n is similar, except that the last element is a 1 and the first element is 0.
e_n = NewArray(n, 1, 0#)
e_n(n, 1) = 1#

' n_1 is an n by n-1 sparse matrix, with 1s on the *sub*diagonal.
n_1 = NewArray(n, n - 1, 0#)
For i = 1 To n-1
    n_1(i+1, i) = 1#
Next i

' n_n is also n by n-1 and sparse, but with 1s on the diagonal.
n_n = NewArray(n, n - 1, 0#)
For i = 1 To n-1
    n_n(i, i) = 1#
Next i

    The only duplication here is due to using two for loops, which you could easily combine if that really bothered you enough.

  • Kashim (unregistered)

    e_1 = NewArray( n, 1, 0# ): e_n = NewArray( n, 1, 0# )

    n_1 = NewArray( n, n-1, 0# ): n_n = NewArray( n, n-1, 0# )

    e_1( 1, 1 ) = 1#

    e_n( n, 1 ) = 1#

    For i = 1 To ( n - 1 )

    n_n( i, i ) = 1#

n_1( i + 1, i ) = 1#

    Next i

    Seriously. I also sent this in via the link with my stuff attached just in case I win, but this is why that is a bad pattern. TRWTFs: Doing things in loops that need to be done exactly once, Doing what is clearly going to be Matrix Math without a matrix library, thinking that there isn't a better solution, using variables to reference things that will never move (See their method for e_n using (i , 1) instead of just (1,1)),

    Anything that can be done with For-Case can be done in other, better ways. There are times when it get shoehorned into an existing system because it's easy as heck to add and relatively easy to understand, but that doesn't make it good. If I'd been asked to review this code and it was in production, I would likely not have given more than a sideways glance. It isn't the worst anti-pattern in this case, but it is NOT good.

  • Geek (unregistered)

    Site admin definitely needs to upgrade the Captcha - too many adbot intrusions.

  • (nodebb) in reply to Geek

    Yeah, adbots with hernias, no less. (Check the domains in the links.)

  • John Wallace (google)

    That was fun:

    [code] /* allocate all 0's / arr = calloc(nn-1, sizeof(int));

    /* set appropriate elements to 1 / for(int i=1; i<n; i++){ arr[in-1] = 1; }

    /* set the variables */ e_1 = n_n = &arr[n-1]; e_n = n_1 = &arr[0]; [\code]

    Pure evil, but It Works! The best part is that it looks like it might be reasonable (and the author would crap on any objections because it's "so creative and elegant").

  • BTGuy (unregistered)

    I was working in PHP today, so thought I'd do a little practice.

    My rough, quick answer: function DefineProjectionArrays($n, &$e_1, &$e_n, &$n_1, &$n_n) { if ($n < 2) { return; } $e_1 = $e_n = array_fill(0, $n, array_fill(0, 1, 0)); $n_1 = $n_n = array_fill(0, $n, array_fill(0, $n - 1, 0));

    $e_1[0][0] = $n_n[0][0] = $e_n[$n-1][0] = $n_1[$n-1][$n-2] = 1;
for ($i = 0; ++$i < $n - 1;) {
	$n_1[$i][$i-1] = 1;
	$n_n[$i][$i] = 1;
}

    }

    And some creatively worse solutions: function DefineWorseProjectionArrays($n, &$e_1, &$e_n, &$n_1, &$n_n) { $e_1 = $e_n = $n_1 = $n_n = array();

    for ($i = -1; ++$i < $n;) {
	$e_1[] = $e_n[] = array(0);
	$zeros = array();
	for ($j = -1; ++$j < $n - 1;) {
		$zeros[] = 0;
	}
	$n_1[] = $n_n[] = $zeros;
}
$e_1[0][0] = $n_n[0][0] = $e_n[$n-1][0] = $n_1[$n-1][$n-2] = 1;
for ($i = 0; ++$i < $n - 1;) {
	$n_n[$i][$i] = 1;
	$n_1[$i] = $n_n[$i];
	array_shift($n_1[$i]);
	$n_1[$i][] = 0;
}

    }

    function DefineEvenWorseProjectionArrays($n, &$e_1, &$e_n, &$n_1, &$n_n) { $e_1 = $e_n = $n_1 = $n_n = array();

    for ($i = -1; ++$i < $n;) {
	$e_1[] = $e_n[] = array(0);
	$zeros = array();
	for ($j = -1; ++$j < $n - 1;) {
		$zeros[] = 0;
	}
	$n_1[] = $n_n[] = $zeros;
}
$e_1[0][0] = $n_n[0][0] = $e_n[$n-1][0] = $n_1[$n-1][$n-2] = 1;
for ($i = 0; ++$i < $n - 1;) {
	$n_n[$i][$i] = 1;
	$n_1[$i] = array_map('intval', str_split(str_pad(decbin(bindec(implode("", $n_n[$i])) << 1), $n - 1, "0", STR_PAD_LEFT)));
}

    }

  • Anonymous (unregistered)

    The real WTF is that this code fails for n = 1. If my mental VB interpreter is correct, this code returns [1], [0], [[]], [[]] instead of [1], [1], [[]], [[]].

  • downstevedown (github)

    I like the fact that this example is written in an old enough version of VB to preclude the option of declaring 4 arrays with initializers. These arrays are small enough that they don't really need code to initialize them.

  • (nodebb) in reply to phate

    You just need to create a nxn identity matrix and slice it properly... like, in python (I don't know VBA, but this shouldn't be that hard...):

    tmp = numpy.identity(n)

    e_1 = tmp[:,0]
    e_n = tmp[:,n-1]
    n_1 = tmp[:,1:n-1]
    n_n = tmp[:,0:n-2]

    Nowhere near Pythonic enough. Plus, e_1 and e_n are supposed to be two-dimensional. You want this:

    tmp = numpy.identity(n)
e_1 = tmp[:,:1]
n_1 = tmp[:,1:]
e_n = tmp[:,-1:]
n_n = tmp[:,:-1]
  • Tinkle (unregistered)

    Obfuscation level 2 (level 1 is the original WTF)

    Public Sub DefineProjectionArrays(ByVal n As Long, ByRef e_1 As Variant, ByRef e_n As Variant, ByRef n_1 As Variant, ByRef n_n As Variant) Dim i As Long, j As Long, k as Long

    k = n - 1

' Fill arrays with zeros
e_1 = NewArray(n, 1, 0#)
e_n = NewArray(n, 1, 0#)
n_1 = NewArray(n, k, 0#)
n_n = NewArray(n, k, 0#)

For i = 0 To n - 1
    For j = 0 To k - 1
        ' Store row and column. Use \Mod to retrieve them.
        n_1(i + 1, j + 1) = i * k + j;
        n_n(i + 1, j + 1) = j * n + i;
    Next
Next

' Set unit elements according to the pattern above
For i = 1 To n
    Select Case i
        Case 1:                         ' first row of e_1 and n_n have values
            e_1[i, 1] = 1;
            For j = 1 to k
                n_n(i, j) = Iff(((n_n(i, j) \ n) + 1) = i, 1#, 0#)
                n_1(i, j) = Iff((n_1(i, j) \ k) = j), 1#, 0#)
            Next
        Case n:                         ' last row of e_n and n_1 have values
            e_n(i, 1) = 1
            For j = 1 To k
                n_1(i, j) = Iff((n_1(i, j) / k) = j), 1#, 0#)
                n_n(i, j) = Iff((n_n(i, j) / n) = (i - 1), 1#, 0#);
            Next
        Case Else                       ' middle rows of n_1 and n_n have values
            For j = 1 To k
                Select Case j
                    Case n_n(i, j) % n + 1:
                        n_n(i, j) = 1
                    Case Else
                        n_n(i, j) = 0
                End Select
                n_1(i, j) = Iff((n_1(i, j) % k + 1) = (i - 1), 1#, 0#) 
            Next
    End Select
Next

    End Sub

Leave a comment on “A Foursome of Arrays”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article