Visual Basic stupidness



  •  Visual Basic is BASIC. So is the IQ of the developers that use it most of the time.

    All these monstrosities from the Microsoft.VisualBasic namespace...

     And this code:

     

    	Private Shared Function IsQuarter(ByVal month As Integer) As Boolean
    
    	Select Case month
    		Case 1
    			Return False
    		Case 2
    			Return False
    		Case 3
    			Return True
    		Case 4
    			Return False
    		Case 5
    			Return False
    		Case 6
    			Return True
    		Case 7
    			Return False
    		Case 8
    			Return False
    		Case 9
    			Return True
    		Case 10
    			Return False
    		Case 11
    			Return False
    		Case 12
    			Return True
    	End Select
    
    End Function</pre>
    

    How long can you make it?...

    What's wrong with this: short, plain and simple

    	Function Mod3(ByVal number As Integer) As Boolean
    		Return (number Mod 3 = 0)
    	End Function
    

    Good thing I quit



  • That's the worst that you could find? Lucky you.


  • Discourse touched me in a no-no place

    @Snake said:

    What's wrong with this: short, plain and simple
    The function name for starters. And the fact that it gives different results to the original for a whole range of values. Not that there aren't a few things wrong with the original...



  • This is what happens when you ship a development tool with your product. Everyone can program it. And, may I say, it does what it says and is easy to read? This would be how non-programmers think, probably. The problem would be when this program becomes "Mission Critical" and now has to be supported. Still, in this case it is very easy to see what the original "programmer" was trying to achieve. Even named it well. Better than some things I have seen

     



  •  There is no use in developing a function that does more than its supposed to do. I KNOW I only get a value 1 -> 12. So it doesn't really matter :) 

     



  • @Snake said:

     There is no use in developing a function that does more than its supposed to do. I KNOW I only get a value 1 -> 12. So it doesn't really matter :) 

     

     

    The original function doesn't do more than it's supposed to do, it just has a relatively inefficient implementation.

    Your function:

    - is poorly named
    - makes itself redundant by virtue of its name
    - returns completely wrong values for invalid input.

    So I'm rather sorry, but your solution is inferior. I reject your check-in into the repository. Please report to your code review supervisor this afternoon.



  •  Agreed, the original method makes sense, is easily readable, and works well. If there's a problem with it, it's easy to see where it is. If there's a problem with the OP's, you have to first figure out what the modulus is doing and why, and then figure out what the modulus should be.



  • @Snake said:

    Visual Basic is BASIC. So is the IQ of the developers that use it most of the time.
    I'm sorry; I can't let this stand.  The IQ of the developers is basic?  I hope that construction sounded a lot better in your head, because it doesn't make any sense here in reality.


  • ♿ (Parody)

    @bstorer said:

    @Snake said:
    Visual Basic is BASIC. So is the IQ of the developers that use it most of the time.
    I'm sorry; I can't let this stand.  The IQ of the developers is basic?  I hope that construction sounded a lot better in your head, because it doesn't make any sense here in reality.
    Maybe he meant they were greater than 7?



  • @bstorer said:

    The IQ of the developers is basic?
    No the IQ of the devoplers is BASIC. Which has an lrc value of 90. As does Basic, but implying that their IQ is basic is certainly a horrible thing to say. No-one with an IQ of 122 would ever touch such an ambominable language as VB.



  •  The function works, does what it's supposed to, and works correctly even for invalid values. Yours doesn't.

    'Nuff said.



  • @Snake said:

    What's wrong with this: short, plain and simple

    	Function Mod3(ByVal number As Integer) As Boolean
    		Return (number Mod 3 = 0)
    	End Function
    
    Private Shared Function IsQuarter(ByVal month As Integer) As Boolean
    		Select Case month
    			Case 3
    				Return True
    			Case 6
    				Return True
    			Case 9
    				Return True
    			Case 12
    				Return True
    		End Select
    		Return False
    	End Function
    IMO that's cleaner. Only difference between mine and the original is it'll return False for out of bounds values, I haven't a clue what VB would normally return. I'd also use an if statement, but I don't feel like looking up how much VB fucked up such a simple thing.


  • @Lingerance said:

    Private Shared Function IsQuarter(ByVal month As Integer) As Boolean
    		Select Case month
    			Case 3
    				Return True
    			Case 6
    				Return True
    			Case 9
    				Return True
    			Case 12
    				Return True
    		End Select
    		Return False
    	End Function

    Or, make it even shorter...

    Private Shared Function IsQuarter(ByVal month As Integer) As Boolean
    		Select Case month
    			Case 3,6,9,12
    				Return True
                            Case Else
                                    Return False
    		End Select
    	
    End Function
    

    Whatever the case... I think the main point is using some clever little math formula only works if it makes sense. You can't just say, "well, I don't expect myself to use this function for anything but this specific subset of inputs" The first function works fine- it just had a few extraneous cases. The fact that they didn't try to be a clever coder isn't a wtf, in fact, that's a good thing. it means when they switch to C# you won't have to deal with 50 nested ternary operators with esoteric math in them all in the name of "being clever". You could also have said

    return (month / 3) - Int(month \ 3) = 0

    But it still returns the wrong value for out of range values, and unless you have the good sense to give the function a name a little more meaningful then mod3 (exactly what it does "mod3"? seriously? what will the comments be? "returns wether the number divided by three has a remainder of 0"?)

    Yes- the original code could be "cleverafied" but as others have stated theirs is a lot easier to understand and comes with the benefit of actually being named for it's intent, which will make any code referring to this function a lot easier to read then if it were called "mod3".

    I call "not a WTF" or possibly that TRWTF is the OP.



  • Informative method name, logic that clearly does what it claims (albeit in a somewhat inefficient manner). It's not a bad function. Whereas yours, as others have pointed out, does not describe what it's supposed to do, nor even what it ACTUALLY does (it should be called IsMod3, because Mod3 just sounds like it's performing % 3 and returning the result). Imagine coming in and looking at your function and trying to figure out why the hell it was there. And to top it off, your function is nothing more than a call to a couple of operators--a completely useless wrapper around one line of code.

    @Snake said:

    Good thing I quit

    I agree. Now the company doesn't have to pay you severance.



  • @Lingerance said:

    @Snake said:

    What's wrong with this: short, plain and simple

    	Function Mod3(ByVal number As Integer) As Boolean
    		Return (number Mod 3 = 0)
    	End Function
    
    Private Shared Function IsQuarter(ByVal month As Integer) As Boolean
    		Select Case month
    			Case 3
    				Return True
    			Case 6
    				Return True
    			Case 9
    				Return True
    			Case 12
    				Return True
    		End Select
    		Return False
    	End Function
    IMO that's cleaner. Only difference between mine and the original is it'll return False for out of bounds values, I haven't a clue what VB would normally return. I'd also use an if statement, but I don't feel like looking up how much VB fucked up such a simple thing.
    Or even:

    Private Shared Function IsQuarter(ByVal month As Integer) As Boolean
    		Select Case month
    			Case 3, 6, 9, 12
    				Return True
                            Case is <1, is >12
                                    Return FileNotFound
    			Case Else
    				Return False
    		End Select
    	End Function


  • @Snake said:

     There is no use in developing a function that does more than its supposed to do. I KNOW I only get a value 1 -> 12. So it doesn't really matter :) 

     

    Snake, I think you're quickly finding that the posters here much, much prefer clean code over "clever" code. "Clever" code, even when it works, is far more likely to lead to bugs in the future. (Especially when you give the function a name like "mod3". Sure, you know exactly what it does and what input limits it has, but the next guy (who, likely as not, is going to be worse than you) may not, and when he mis-uses the function, you've just contributed to causing an error in the product.

    I know I'm just hopping on the bandwagon here, sorry. But I hope you're checking back to this thread and perhaps learning something about software development. The most important thing to learn? KISS: Keep It Simple, Stupid!



  • I think Snake made the assumption that this was a generic VB bashing forum.  Admittedly, that's an easy assumption to make :P.

     

    But couldn't this function be replaced by a simple REGEX expression in an embedded device.



  • @Medezark said:

    But couldn't this function be replaced by a simple REGEX expression in an embedded device.

    Only if you used XML.



  •  It could be worse, they could have used a Months enumeration too!



  • My IQ has been measured at 140 and I use VB. I've also made a pretty decent living developing in, and supporting, MS Access. Bad developers have ensured my job security.

     



  • @Lingerance said:

    No-one with an IQ of 122.3 would ever touch such an ambominable language as VB.
    ADTFY



  • @powerlord said:

     It could be worse, they could have used a Months enumeration too!

     

    Or, a bloated JPEG with poor compression and aliased type.



  • Can I suggest a compromise:

    Private Shared Function IsQuarter(ByVal month As Integer) As Boolean
      If month<1 Or month>12
        Trace.Write "I KNOW I only get a value 1 -> 12. Visual Basic is BASIC. So is the IQ of the developers that use it most of the time."
        Trace.Write "In other news, apparent the sky should be falling as IsQuarter function was called with in invalid input, which is umpossible."
        Return False
      End If
      Return (month Mod 3 = 0)
    End Function
    

    Are we all happy now?


  • Discourse touched me in a no-no place

    @nat42 said:

    Are we all happy now?
    Nope. It's not obvious <font color="orange">to VB programmers</font> in this new version what needs to change if quarters don't fall on March, June, September and December. (Say, f'rinstance, your financial year starts in August.)



  • @PJH said:

    @nat42 said:
    Are we all happy now?
    Nope. It's not obvious <FONT color=orange>to VB programmers</FONT> in this new version what needs to change if quarters don't fall on March, June, September and December. (Say, f'rinstance, your financial year starts in August.)

    Of course, some of the other suggestions have that problem too:

    Private Shared Function IsQuarter(ByVal month As Integer) As Boolean
    ' Added 1 to all values to account for financial year starting in August
    		Select Case month
    			Case 4
    				Return True
    			Case 7
    				Return True
    			Case 10
    				Return True
    			Case 13
    				Return True
    		End Select
    		Return False
    	End Function
    

  • Garbage Person

    @Lingerance said:

    No-one with an IQ of 122 would ever touch such an ambominable language as VB.
    I know a lot of people who would argue that point - there's a whole class of absolutely brilliant programmers who take a ... Decidedly mercenary approach to their trade. No loyalties to anything, neither organization nor technique. Just a list of available skills and assets to leverage in the future.

    It's great to be making boat loads of money writing high-powered software (or on DoD contracts in Iraq), but when that kind of work isn't available, some times you have to do concert security for Hannah Montana or sign up as a mallcop until it is.



  • @dhromed said:

    - returns completely wrong values for invalid input.

     

    Interesting... what's the right value for invalid input then? (imho exception, but that's not a value...) why is false better than random() in your opinion?



  • @viraptor said:

    Interesting... what's the right value for invalid input then? (imho exception, but that's not a value...) why is false better than random() in your opinion?
    The function's name is "IsQuarter", so obviously if it's given a non-month it isn't a quarter.



  • @viraptor said:

    Interesting... what's the right value for invalid input then? (imho exception, but that's not a value...) why is false better than random() in your opinion?
     

    The question is:

    Is 30 a quarter?

    The answer is:

    Nope.

    -> False.

    Predictable.

    I also don't think it's this small function's responsibility to alert the system of fucked input. It's a blue collar function, if you will. If there's invalid input given, that should probably have been verified elsewhere in the system, and that function can throw an exception.



Log in to reply