Skip to main content

PL/SQL Brain Teaser: Find all the hard-codings!

We all know that hard-coding is a bad thing (well, maybe not all of us. At one training several years past, I asked the audience "Does anyone think hard-coding is a good idea?" and one person raised his hand. Um, OK).

You know hard-coding: when you say to yourself "This is never going to change." and so you put the "value" directly in your code, over and over again.

I put "value" in quotes, because many developers think simply of hard-coded literal values when they think of hard-coding. But I think there are many more ways that hard-coding can seep into our programs.

So I invite you to help find all the hard-codings in the procedure below.

Here's the rule: you can only identify ONE HARD-CODING in each comment. I will delete a submission with > 1. It'll be more fun that way. Promise!



I will give everyone a couple of days to submit your ideas, then offer my own views on the subject.


Comments

  1. I'll start with the scale of the l_salary variable. It ought to be driven from the underlying column datatype to which it refers.

    ReplyDelete
    Replies
    1. or maybe you just want to have more strict constraint on precision/scale in this method - who knows? you can't just tell by the code itself ...

      Delete
  2. Line 31: 'Something went wrong!' is hardcoded

    ReplyDelete
  3. HARD-CODING:
    Line 01) PROCEDURE process_employee (department_id IN NUMBER)
    BEST PRACTICE:
    Line 01) PROCEDURE process_employee (department_id IN department.department_id%TYPE)

    ReplyDelete
  4. Line 5: 100 (length of variable) is hardcoded

    ReplyDelete
  5. Line 4: 9, 2 (scale, precision) of variable is hardcoded

    ReplyDelete
  6. Line 10: Hardcoded formula for full name

    ReplyDelete
  7. It's not hard-coding, but I wonder why the cursor is never closed -- even in the exception. That said, using a CURSOR FOR LOOP would eliminate the hard coding implicit in the three variable declarations *and* eliminate problems caused by not closing the cursor explicitly.

    ReplyDelete
    Replies
    1. PL/SQL cursors will be closed automatically when they go out of scope, so it's actually unnecessary. Since the cursor wasn't e.g. a package global variable, it'll be closed without any intervention. (A comment to that effect is usually a good idea though.)

      This doesn't apply to DBMS_SQL however: Those will leak if you don't clean up after yourself explicitly. (Well, but the newfangled conversion of DBMS_SQL cursors to SYS_REFCURSOR adds a wrinkle that I haven't explored.)

      Steven F.: Sounds like a nice blog topic ;D

      Delete
  8. Line 10-12 Database structure is hardcoded

    ReplyDelete
  9. Ah, should have made the rule more clear: only specify hard-coding on a single line or statement. Still, Anonymous did consolidate hard codings on three lines into a single identification of a type of hard-coding, so I publish it! :-)

    ReplyDelete
    Replies
    1. Surely lines 8-12 constitute one statement formatted over multiple lines ending with the semicolon serving as the statement terminator; thus satisfying the rule.

      Delete
  10. 10000000 is hardcoded, there should be one parameter in metadata table say meta_variable having value & name as columns , that should be called and 10000000 should be replaced with that

    ReplyDelete
  11. Line 3: NUMBER data type for l_id variable is hard-coding. Should be employees.employee_id%type

    ReplyDelete
  12. Not a hard-coding, but a third parameter TRUE on the RAISE_APPLICATION_ERROR wouldn't go amiss, else you're burying the actual error that you're supposed to be griping about with the ORA-20918.

    ReplyDelete
  13. Line 19-22 we can imagine a boolean-value function instead of IF

    ReplyDelete
  14. It's not a hard-coding, but entire exception block actually convolutes the actual error message as it will not treat the error, nor will it return to the calling environment the actual message.

    ReplyDelete
  15. Using an old-school explicit FETCH instead of a implicit LOOP is pretty 'ard. It leads to bugs too, as demonstrated - WHEN NOTFOUND should immediately follow the FETCH.

    ReplyDelete
  16. Not hard-coding, but LAST_NAME + FIRST_NAME isn't required.

    ReplyDelete
  17. Line 17: INTO variables - hardcodes the column list. Instead a record variable of cursor%rowtype should be used.

    Sven W.

    ReplyDelete
  18. "WHERE department_id = department_id" seems like a problem. "WHERE department_id = process_employee.department_id", surely?

    ReplyDelete
  19. Instead of declaring data types using %type, and then use bulk collect in fetch loop.

    ReplyDelete
  20. The cursor : it should be parameterised

    ReplyDelete
  21. line 3-5: scope indicator of variables (l_) is a form of hard coding

    ReplyDelete
  22. line 7: the comment has a hardcoded reference to a requirements document

    ReplyDelete
  23. line 21: name must_be_ceo is a hardcoding of a functional requirement. if next week plsql developers finally get to earn more than 10000000 too, the name of the procedure must be changed.

    ReplyDelete
  24. line 24: parameterlist and order is hardcoded, should be named parameters

    ReplyDelete
  25. line 26: by placing the exit here (after processing of the row) you hardcoded that there will always be something to process after a fetch.

    ReplyDelete
  26. line 31: you hardcoded that every exception is ORA -20918, thereby loosing the information that you really want/need

    ReplyDelete
  27. Line#: 19 and 24 salary scale (10000000) is hardcoded

    ReplyDelete
  28. the comma ',' in the select statement.

    ReplyDelete
  29. Hi Steven ,

    I fased the issue . We have to change the sum system privileages to over come for hard code.

    example : shared_pool_size parm

    here we have to chage the parameter eighter TRUE,FALSE, OTHER NUMBERICAL VALUES.

    it will works fine.If we set this as per our requiremnet.

    we can change as hard parsin or parsing by changing this setting.

    it is knowned issue for me. i was fight a lot about this.but forget the sytanx things.
    Regards,
    Bhaskar.

    ReplyDelete
    Replies
    1. Is this plain English? I do not understand this - it may be my clumsiness since English is not my native language. But may I have a translation, please?

      Delete
    2. Cristi, no, I would not call this plain English. I am glad that Bhaskar took the time to submit a comment, but it is a bit difficult to parse his full intent. I believe it has to do with cursor sharing, but maybe not. Bhaskar, would you care to clarify?

      Delete
  30. line 4 hardcoded variable precision

    ReplyDelete
  31. line 5 hardcoded variable precision

    ReplyDelete
  32. line 19 hardcoded condition value.

    ReplyDelete
  33. line 24 harcoded function parameter

    ReplyDelete
  34. line 31 hardcoded exception message

    ReplyDelete
  35. The cursor : in an ideal code, this cursor should'nt exist. the data should be fetched from the table via a function.

    ReplyDelete
  36. Question: if we move a hardcoding (like the IF - as it was mentioned before) into a function, procedure or package wouldn't it be still a hardcoding?

    I think it is hard (even almost impossible) to decide when we have to split our code into smaller pieces and when to stop doing it since simply assembling these pieces would incur a too high cost.

    One benefit of a hardcoding is that it might speed things up. But kindly please understand that I do NOT speak in favor of the hardcoding style of writing the code.

    To post a hardcoding which I consider myself from time to time to use, even the existence of the cursor itself is a hardcoding for the following possible reasons (I do NOT state that the reasons are true in this particular situation):
    1. They might require many context change between PL/SQL and SQL engines; depending what code is "inside" the cursor loop,
    2. They might fool the optimizer that one row is accessed at a time, switching from full table scan to index seek, when a full table scan might have been more efficient; this, again, depends on what we do inside the loop;

    ReplyDelete
  37. I have read the posts above, but I have added some necessary explanations to it so I have put it and I add my consideration to Jay Jay who stated the problem before me.

    ReplyDelete
  38. Many thanks to everyone who submitted comments on this post, both to identify specific hard-codings and introduce interesting thoughts as to the subtleties of resolving them (or even leaving them in place selectively!). I believe that for most of the comments, no further comment is needed; the hard-codings are obvious.

    Here are some reflections on what I would call the less obvious aspects of this code:

    1. Why no close of cursor? Good point. First, there is no need to explicitly close the cursor because the PL/SQL engine will manage that for us (as it is a locally declared cursor). Having said that, since you want your code to answer and not RAISE questions, I suggest including one - or getting rid of the explicit loop altogether. Replace with cursor FOR loop, BULK COLLECT, etc. Thanks, Jason, for similar comment and blog encouragement!

    2. Individual, constrained declarations (and linked to that the hard coding on line 17 - that the cursor always returns 3 items - nice catch, svenweller - that is often overlooked): much better to declare a record against the cursor %ROWTYPE (or do so implcitly with cursor FOR loop). If you must declare using a constrained type, then consider doing so with a SUBTYPE - check out this video: https://youtu.be/jZW-uLb52xk

    3. David Aldridge: yes, good catch on that awful WHERE clause - that’s what happens when you don’t have good naming conventions for parameters and/or do not qualify references to variables/parameters inside SQL statements with their PL/SQL scope name, as in

    WHERE department_id = process_employee.department_id

    4. Naming convention (l_) is a hard-coding of the scope (from Erik van Roon): Hmmm. That one gives me pause for sure. My naming conventions are so deeply engrained I would never have thought this. So the idea is that if someday I decide to move the variable from local to global scope I would have to change the name. Again: hmmmm. What do others think of this?

    I am thinking this is a benefit not a problem. It makes me think twice about making such a move, which is almost always the wrong one (going local to global, anyway).

    5. Line 26 containing the EXIT WHEN: great catch! A bug, not so much hard-coding, but something that can esily escape attention.

    6. RE: Cristi’s comments on “move IF to function…still a hardcoding?” Well, at some point you reach “bedrock” - the “hard” aspects of your code (values, rules, relationships) must be expressed in code. The way I approach it is: is this location the ONLY place it appears? Can it be universally available to developers from this single point of definition? And can that be done efficiently? If the answer is yes, then that’s as good a place as any to keep that logic.

    Happy holidays, everyone!

    ReplyDelete
  39. Steven, fully agree.
    I definitely think it is a very good thing to show the scope in the identifier name, and always do so myself.

    But although it may be a form of hardcoding I don't want to do without it's still a form of hardcoding.
    And since the assignment was to find all hardcoding, not to identify stuff I would do differently, it's in the list.
    :)

    ReplyDelete
  40. I'm surprised you don't call the SQL itself hardcoding. I thought that was part of your best practice to move it to its own function. Sure, there is not much else here, but there is some and this table ought to be accessed from other places. Or have you changed you mind on weather SQL is hardcoding?

    Also, the construction of the name (lname) is an unneeded hardcoding as it is never used.

    ReplyDelete
  41. I posted a brief note on naming conventions for PL/SQL source code here: https://blogs.oracle.com/plsql-and-ebr/resource/Response_To_15_Dec_2015_Plsql_Brain_Teaser.pdf

    ReplyDelete
  42. For the people who got interested after Bryn's response, I would suggest reading the following blog post:

    https://oracle-base.com/blog/2015/11/25/plsql-formatting-more-pearls-of-wisdom-from-bryn/

    Proved to be a real eye opener for me and gave me something to think about hard ;-)

    ReplyDelete
  43. Without reading any comments or answers, the "hard-coding" I see is:
    1. - The datatype specification for the variable l_salary with a hard-code precision and scale.

    2. - The concatentation of last_name and first_name column with a string of a comman in quotes. That could've been made a variable.

    3. - The boolean expression in the IF statement comparing the salary to the number 10000000

    4. - The user-defined ORA-error number used in the call to raise_application_error

    5. - The user-defined ORA-message used in the call to raise_application_error

    ReplyDelete
  44. Sorry for Breaking the rule of "One Hard Coding Per Comment".
    Boss, Every line has hard coding.
    Except these lines.

    2 , 9 , 13 , 16 , 20 , 22 , 27 , 28 , 29 , 30 , 32

    ReplyDelete
  45. This comment has been removed by the author.

    ReplyDelete
  46. In Declaration two hard coding is there.
    1. L_salary number(9,2) -- we can declare as number (with type)
    2. L_name varchar2(100) – 100 is hard coding
    3. Name concatenation – ‘,’
    4. L_salary > 100000 -- we can include in input (10000)
    5. Raise application_error - error code and description is hard coded

    ReplyDelete

Post a Comment

Popular posts from this blog

Quick Guide to User-Defined Types in Oracle PL/SQL

A Twitter follower recently asked for more information on user-defined types in the PL/SQL language, and I figured the best way to answer is to offer up this blog post. PL/SQL is a strongly-typed language . Before you can work with a variable or constant, it must be declared with a type (yes, PL/SQL also supports lots of implicit conversions from one type to another, but still, everything must be declared with a type). PL/SQL offers a wide array of pre-defined data types , both in the language natively (such as VARCHAR2, PLS_INTEGER, BOOLEAN, etc.) and in a variety of supplied packages (e.g., the NUMBER_TABLE collection type in the DBMS_SQL package). Data types in PL/SQL can be scalars, such as strings and numbers, or composite (consisting of one or more scalars), such as record types, collection types and object types. You can't really declare your own "user-defined" scalars, though you can define subtypes  from those scalars, which can be very helpful from the p

The differences between deterministic and result cache features

 EVERY once in a while, a developer gets in touch with a question like this: I am confused about the exact difference between deterministic and result_cache. Do they have different application use cases? I have used deterministic feature in many functions which retrieve data from some lookup tables. Is it essential to replace these 'deterministic' key words with 'result_cache'?  So I thought I'd write a post about the differences between these two features. But first, let's make sure we all understand what it means for a function to be  deterministic. From Wikipedia : In computer science, a deterministic algorithm is an algorithm which, given a particular input, will always produce the same output, with the underlying machine always passing through the same sequence of states.  Another way of putting this is that a deterministic subprogram (procedure or function) has no side-effects. If you pass a certain set of arguments for the parameters, you will always get

My two favorite APEX 5 features: Regional Display Selector and Cards

We (the over-sized development team for the PL/SQL Challenge - myself and my son, Eli) have been busy creating a new website on top of the PLCH platform (tables and packages): The Oracle Dev Gym! In a few short months (and just a part time involvement by yours truly), we have leveraged Oracle Application Express 5 to create what I think is an elegant, easy-to-use site that our users will absolutely love.  We plan to initially make the Dev Gym available only for current users of PL/SQL Challenge, so we can get feedback from our loyal user base. We will make the necessary adjustments and then offer it for general availability later this year. Anyway, more on that as the date approaches (the date being June 27, the APEX Open Mic Night at Kscope16 , where I will present it to a packed room of APEX experts). What I want to talk about today are two features of APEX that are making me so happy these days: Regional Display Selector and Cards. Regional Display Sel