From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on polar.synack.me X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00 autolearn=ham autolearn_force=no version=3.4.4 X-Google-Language: ENGLISH,ASCII-7-bit X-Google-Thread: 103376,84a9e3abc73c226f X-Google-Attributes: gid103376,public From: mfb@mbunix.mitre.org (Michael F Brenner) Subject: Re: Strings in Ada83 Date: 1997/10/15 Message-ID: <622h8m$bd1@top.mitre.org> X-Deja-AN: 280731333 References: <3443D8AF.D9CD4393@morden.csee.usf.edu> Organization: The MITRE Corporation, Bedford Mass. Newsgroups: comp.lang.ada Date: 1997-10-15T00:00:00+00:00 List-Id: Scott said: > ... strings package is at > http://www.csee.usf.edu/~dick/cstring.html > ... your comments would be most welcome. Thanks... If you ever choose to revise it, you might consider these ideas: (1) Separation of I/O from computation. For example, The get_line and put_line could be put in a second package (or a child package in Ada-95). With suitable revisions, the package would then be a pure package which would be subject to additional optimizations. In Ada-95 this pureness can be celebrated with a specific pragma, but an Ada-83 optimizer can still do the data analysis to recoginize the pureness of the code. Making the string manipulation portion of the package dependent upon text_io makes it non-portable. Realtime embedded systems may not have a text_io connected, consider a satellite in orbit. In addition, text_io usually brings in a lot of software which is a memory hog. More important than the practical problems with forcing the user to use text_io in order to use strings is the lack of cohesion in the package caused by mixing I/O with string manipulation; to increase the cohesion, let the package do one thing (string manipulation) without a very complex other thing (I/O). (2) Use of a limited type. Since you chose to make it a limited type, why not make INSERT and DELETE functions instead of procedures, taking advantage of a fully functional style? (3) Use of a limited private type. Consider making type cstrings a limited private type, and convert all the functions to procedures, except tostring, length, and the comparision operators, taking advantage of the safety of limited private thypes. (4) Letting the user choose the size of each string. You might wish to change the type definition to permit each string to have its own size. Arrays, however, would have to have all strings of the same maximum length. (5) The exception cstring_null which is raised when the text_io file is not open, does not inform the user what the real problem is. (6) Have you tested this using the full capability of text_io to process files with end-of-page markers in them? (7) The loop in the less-than function would be easier to read, by removing the boolean variable GO and experiencing the corresponding lowering in the depth of nesting. To do this, replace the statement IF LEFT... /= RIGHT ... with the statement EXIT WHEN LEFT /= RIGHT. In most compilers, doing this would result in faster code because the I:=I+1 would then be the innermost nesting level of the loop and thus be slightly better optimized. This would not matter so much in a machine with a deep instruction pipeline or where the program cache is not voided by jumps. (8) The body of the function less-than-or-equal-to can be simplified by noting that IF CONDITION THEN RETURN TRUE ELSE RETURN FALSE is equivalent to RETURN CONDITION. (9) There are no protection against uninitialized cstrings (as you and this code may go into long or infinite loops when used on uninitialized cstrings. (10) Returning a maximum sized Ada string (say 1000000 characters long) for a 1 character long cstring is not efficient and this should be documented for the user. Better, why not return an Ada string that is the same length as the cstring? (11) In CSTRINGTONUMBER, the expression IF NOT (('0'<=X) and (x<='9')) THEN would be more readable if replaced with IF X NOT IN '0'..'9' THEN (12) The exception cstring_null in cstringtonumber does not adequately inform the user that a minus sign, plus sign, underline, or decimal point was used as part of a number and this code prohibits those characters. (13) It might be worth testing why MY_REVERSE does not actually reverse all cstrings. (14) The variable FLAG would be obviated by replacing flag:=True; WHILE FLAG LOOP ... code ... if CONDITION then flag := FALSE; end if; end loop with the simpler loop loop ... code ... exit when CONDITION; end loop; (15) When back filling with ASCII null characters, some of the loops fill the cstring up to max_size and some fill it up to max_size+1. This is often the sign of a bug. In this case, using suitable checks for non-initialized cstrings, the appropriate thing to do is not to fill the cstrings at all, but just add a single ASCII null at then end of the meat of the string. (16) The formal parameter INDEX could be renamed AFTER to indicate its function in the procedure INSERT. (17) What happens when inserting something at the beginning of a cstring? (18) What happens when deleting max_size characters from a string? (19) Is it true that BM_scan only located strings correctly when the pattern string being looked for (SUB) does not contain repetitious strings withing itself? For example, will it fail if INJINKINL is searched using the pattern INKINL, because it notices that INKINL doesnot match INJINK (the first 6 letters) and bumps up the count by 6, next trying to match INKINL against INL which also fails? (20) The test programs in your sample drivers do not flesh out all the bugs. An IF statement is needed for each feature of the package. This should only appear inside this package if it done entirely with ASSERT statements and use no I/O. procedure ASSERT (condition: boolean) is Scott_made_an_error: exception; begin if not condition then raise Scott_made_an_error; end if; end assert; (21) The data integrity paragraph of the user documentation is not accurate, promising that cstrings can only be accessed through the operations defined in the package. In addition, a private type can be access through the equality operator, the assignment operator, and through non-initialized elaboration. (22) The Notes on Design and Implementation are not adequate to fix the bugs or to maintain this software. (23) The above comments, notwithstanding, this is a good piece of work and, if you concentrate an equal amount of time writing test programs as you do writing lines of code, you will solve many problems during your long and happy career.