Question UPDATE statement not working

M

Mady

Member
Hi All,

Seeking some conceptual advice here. Somehow UPDATE statement is not showing up when I run the attached query. Initially, I was using ASSIGN statement and it was working fine. But as I wanted to see the data on screen and then update it, I used UPDATE statement (Line#115) instead but its does not work. Is this a frame control issue ?
Please advice. Thanks.
 

Attachments

1646861693814.png

My first suggestion, to make your code more comprehensible to yourself and others, is to work on your indentation and code style; they are a part of your coding standards, along with capitalization, line-breaking, use of whitespace, etc. Frankly, the indentation is all over the place and I can't understand why many lines have the indentation that they do. Indentation should convey a sense of the block structure of the code, where you have conditional execution, etc.

Consider the last five lines of the screenshot above. There the indentation makes sense to me. You have a condition (IF NOT AVAILABLE binmst THEN) followed by a DO block with two indented statements. Their indentation visually conveys that they are within that block and subject to that condition. But the first visible IF statement (IF NOT AVAILABLE ITEM...) uses similar indentation but puts the line break after the DO: rather than between THEN and DO:. This inconsistency adds to the cognitive load on the reader.

You have a couple of instances of indenting the statements following a FIND statement, as if it were the beginning of a block like DO, FOR, or REPEAT; it is not.

Looking at your REPEAT block near the top, you indent the following IMPORT statement by 4 spaces, then the following ASSIGN (which should not be further indented) by 8 spaces, then the following DO statement (which also should not be further indented) by a further 7 spaces. Then the next two statements, despite being within that DO block, are not indented at all. It is a bit of a mess.

You might be inclined to argue that all of these criticisms are about cosmetic things and they do not affect the code that the compiler emits. That's true. But you and I, and whoever is given that task of maintaining this code after you, are not compilers. Coding standards exist for a reason. They ensure consistency and comprehension. They facilitate static analysis. They are important.

Certainly there are many differences between one coding standard and another, and people much more knowledgeable than me will argue their relative merits. The point here is that having just about any coding standard is better than having none.

So while you consider this one particular case (what is happening with my UPDATE statement?), take a step back and think about how this code, whether semantically correct or not, could be made much more readable and thus easier to debug. And think about whether that problem is limited to this one particular procedure or whether it is much larger in scope within your application.
 
@Mady putting my money where my mouth is, here is your code (with the same logic) with some formatting and styling applied. It may not be everyone's cup of tea, but again, just about any standard is better than none. See if this makes it a little easier for you to read.
Code: 
define variable icount          as integer   no-undo.
define variable v-whse          as character no-undo.
define variable clist           as character no-undo.
define variable v-count-updated as integer   no-undo.
define variable v-count-total   as integer   no-undo.
define variable v-subject       as character no-undo.
define variable v-emailbody     as character no-undo.
define variable v-filename      as character no-undo.
define variable v-filepath      as character no-undo.
define variable v-recipient     as character no-undo format "x(25)".
define variable v-email         as character no-undo format "x(25)".

/* defining a variable LIKE a database field 
   tightly couples your code to your schema
   and may introduce side-effects; it is not
   a best practice
 */
define variable v-co_num        like ITEM.co_num no-undo.
define variable v-wh_num        like ITEM.wh_num no-undo.
/* no-undo missing for v-abs_num? or intentionally omitted? if the latter, document/comment it */
define variable v-abs_num       like inventory.abs_num. 

define stream strnotavlbl.
define stream strbackup.

define buffer binventory for inventory.

update "Input Data: " v-co_num v-wh_num v-email with frame a.

assign 
  v-subject   = "inventory move update logs"
  v-emailbody = "inventory update logs"  
  v-filename  = "move_logs.txt"
  v-filepath  = "/ad/move_logs.txt"
  .

input from "/ad/testdata.csv".

output stream strbackup   to "/ad/move_backup.txt".
output stream strnotavlbl to "/ad/move_logs.txt".

import clist.

repeat:

  import unformatted clist.

  v-whse = trim(entry(1,clist)).
  
  do icount = 1 to num-entries(v-whse):
                   
    v-count-total = v-count-total + 1.
                   
    find first item
      where ITEM.co_num  = v-co_num 
        and ITEM.wh_num  = v-wh_num
        and ITEM.abs_num = entry(2,clist) no-lock no-error.
      
    if not available ITEM then 
    do:
      export stream strnotavlbl delimiter "," "Item Master not available-" clist.
      next.
    end.
 
    else 
      do:
        find first binmst 
          where binmst.co_num  = ITEM.co_num
            and binmst.wh_num  = ITEM.wh_num 
            and binmst.bin_num = entry(3,clist) no-lock no-error.
          
        if not available binmst then 
          do:
            find first binmst 
              where binmst.co_num  = ITEM.co_num
                and binmst.wh_num  = ITEM.wh_num 
                and binmst.bin_num = entry(4,clist) no-lock no-error.
              
            if not available binmst then 
              do:
                export stream strnotavlbl delimiter "," "Bin Master Record not available for both bins-" clist .
                next.
              end.
          
            else 
              do:      
                export stream strnotavlbl delimiter "," "Bin Master Record not available for first bin-" clist .
                next.
              end.
              
          end.
        
        else 
          do:
            find first binmst 
              where binmst.co_num  = ITEM.co_num
                and binmst.wh_num  = ITEM.wh_num 
                and binmst.bin_num = entry(4,clist) no-lock no-error.   
                
            if not available binmst then 
            do:
              export stream strnotavlbl delimiter "," "Bin Master Record not available for second bin-" clist .
              next.
            end.
        
            else 
              do:
                find first inventory 
                  where inventory.co_num  = binmst.co_num
                    and inventory.wh_num  = binmst.wh_num
                    and inventory.abs_num = ITEM.abs_num
                    and inventory.bin_num = entry(3,clist) exclusive-lock no-error.
                
                if not available inventory then 
                do:
                  export stream strnotavlbl delimiter "," "Inventory Bin record not available-" clist .
                  next.
                end.
                
                else 
                    do:
                    v-count-updated = v-count-updated + 1.
                    
                    export stream strbackup delimiter "," 
                      inventory.co_num 
                      inventory.wh_num 
                      inventory.abs_num
                      inventory.bin_num
                      .
                    
                    /*assign inventory.bin_num = entry(4,clist).*/
                    update inventory.bin_num.
                  end. 
                      
              end.  /* else do */
              
          end.  /* else do */
      
      end.  /* else do */
  
  end.  /* do icount... */
  
end.  /* repeat */
 
export stream strnotavlbl "Total Records in the File - " v-count-total.
export stream strnotavlbl "Total Records Updated - " v-count-updated.

output stream strnotavlbl close.
output stream strbackup   close.

run /ad/sendmail.p(input v-email,       /* recipient email address */
                   input v-subject,     /* subject line            */
                   input v-emailbody,   /* body of the email       */
                   input v-filename,
                   input v-filepath).

/* Clean up...delete the file */                     
if search(v-filepath) ne ? then
  unix silent rm value(v-filepath).

Also, your choice of tools can greatly affect your ability to read and debug your code. If you use an IDE, or even a good text editor, that gives you options like code folding and syntax highlighting, it can really help with making sense of many nested blocks like you have here.

If I have time later, I might even try to figure out your reported problem with your update statement. :)
 
:) .. Thank you very very much Rob .. I also took your comments seriously and went to improve the formatting first and then tried to understand the issue. I could not find the reason, but it is something to do with the update statement.
I just thought of trying this with Temp-Table - Created a temp table and imported all the csv records. Using this approach, I was able to use UPDATE statement without any issue. Attached is my latest file.
I am using PIEW - but its formatting is bad, yours is too good.
 

Attachments

Some thoughts from my formatting:
  • I prefer lower-case keywords, as you can see. You may not. Whatever you prefer, do it consistently.
  • Don't take shortcuts, e.g. by abbreviating keywords (e.g. "var" instead of "variable", or omitting a table name when referencing a field).
  • Your indenting used a mix of tabs and spaces. I never use tabs for spacing. I know this is a religious topic for some, but for me the bottom line is that a space always looks like one space whereas a tab may look like any number of spaces, depending on the logic or configuration of the application that is interpreting the source file. So my advice is this: if you are the only person who will ever see or edit your code, use what makes you happy. Otherwise, use spaces.
  • As I wrote in my comments, I suggest you avoid defining a variable LIKE a database field, or a temp-table LIKE a database table.
  • Always define variables, parameters, and temp-tables as no-undo, unless you have a specific reason for doing so and it is clearly documented for others to understand.
I don't use PIEW but have a look through its settings. Maybe you be able to configure the styling or formatting to better suit your needs.
 
Mady - I believe your UPDATE isn't working because your INPUT is coming from your csv file. Try defining an input stream to use with your file.
 
@Mady @deanw is correct. Your input is being taken from the CSV file and not the standard input which means that whatever is next to be read in that CSV file is what is going to get stored against inventory.bin_num.
 
Last edited:
You must log in or register to reply here.
Back
Top