![]() |
Forum Index : Microcontroller and PC projects : Tidy programs
Page 1 of 2 ![]() ![]() |
|||||
Author | Message | ||||
vk4tec![]() Senior Member ![]() Joined: 24/03/2012 Location: AustraliaPosts: 239 |
Is it good practice to sub most routines ? Andrew Rich VK4TEC www.tech-software.net |
||||
graynomad![]() Senior Member ![]() Joined: 21/07/2010 Location: AustraliaPosts: 122 |
I'm not familiar with MM basic so I may have interpreted the above incorrectly, but in general it's good practice to use subroutines for any block of code that's used more than once. Rob Gray, AKA the Graynomad, www.robgray.com |
||||
boss![]() Senior Member ![]() Joined: 19/08/2011 Location: CanadaPosts: 268 |
Hi, think yes and yes and yes again. The programs are structured to be easily serviceable . The "main body" could remain intact and most changes can be made in the subroutines. Just in case you are not 100% sure if a new algoritm will work better than old one, you simply add a new subroutine and edit the call command in the main. I'm not a professional programmer, but as a project manager, I'm responible to maintain and keep many old programs running written in different languages (Fortran, C,C++, assembler...). |
||||
djuqa![]() Guru ![]() Joined: 23/11/2011 Location: AustraliaPosts: 447 |
Most definitely. My old Uni. Lecturer said the ultimate program was; Main_Sub(); With everything defines as functions, Routines and parameter files VK4MU MicroController Units |
||||
graynomad![]() Senior Member ![]() Joined: 21/07/2010 Location: AustraliaPosts: 122 |
I've seen simpler, IIRC int i; That's the entire top-level program, all the code was in the overridden int constructor (C++), this is actually bad practice :) Rob Gray, AKA the Graynomad, www.robgray.com |
||||
djuqa![]() Guru ![]() Joined: 23/11/2011 Location: AustraliaPosts: 447 |
During the early days (70's-90's) C and then later C++ programmers were notorious for unreadable and poorly engineered programs; Basic programs looked like a collection of randomly placed statements using arbitrary selected variables with countless (often unneeded) GOTO statements. Pascal at least was designed to be well structured from the beginning.. Software Engineerng is the unit most failed in UNI level Programming courses. There is more to programming than simply stringing declarations, functions together. A program SHOULD be designed before any code is done. 1/ Define what the program will do and why. 2/ Define the tasks needed to achieve desired results. 3/ Design the functions and data needed to implement those tasks. 4/ Now and only now do we CODE. VK4MU MicroController Units |
||||
boss![]() Senior Member ![]() Joined: 19/08/2011 Location: CanadaPosts: 268 |
A little bit theory: Structured programming (sometimes known as modular programming) is a subset of procedural programming that enforces a logical structure on the program being written to make it more efficient and easier to understand and modify. Certain languages such as Ada, Pascal, and dBASE are designed with features that encourage or enforce a logical program structure. Structured programming frequently employs a top-down design model, in which developers map out the overall program structure into separate subsections. A defined function or set of similar functions is coded in a separate module or submodule, which means that code can be loaded into memory more efficiently and that modules can be reused in other programs. After a module has been tested individually, it is then integrated with other modules into the overall program structure. Program flow follows a simple hierarchical model that employs looping constructs such as "for," "repeat," and "while." Use of the "Go To" statement is discouraged. |
||||
shoebuckle Senior Member ![]() Joined: 21/01/2012 Location: AustraliaPosts: 189 |
Agree. I was taught that the first thing to do when you write a program is to turn the computer OFF. It forces you to think about the task. As for subroutines (and functions), yes definitely they should be used. Once debugged, you can forget it and get on with the rest of the problem as it will always do the right thing. e.g. a subroutine to get the next record or next character or print a line. However two things are important for readability and ease of maintenance. 1. Each subroutine should have only one entry point and only one exit. 2. Each subroutine should do only one job. Making one that tries to do several jobs can make maintenance much more difficult later. Yes, it might increase the number of lines of code, but it will make your program much easier to follow and therefore less likely to contain a bug. It is surprising how easy it is to forget why you wrote a piece of code that way. Simple code makes it easy to understand and maintain. One other thing: avoid "magic numbers" in your code. It is much better to use a variable with a meaningful name and assign the value to it than just to use the number in the calculation. Again, this is for readability and maintainability. Too often I have seen code with numbers in it and no explanation of what they represent. Cheers, Hugh |
||||
MicroBlocks![]() Guru ![]() Joined: 12/05/2012 Location: ThailandPosts: 2209 |
I used to audit a lot of software and readability is very important. A computer language written like a spoken language can help make things clear without the need for remarks. Instead of using short variable names use words that you can use. This will tell the reader nothing. [code]If a > 4.5 then[/code] This is much to lengthy [code]If a > 4.5 then 'Check if the pressure is higher then 4.5[/code] use instead: [code]If Pressure > 4.5 then[/code] Seems a simple 'rule' but it is not used enough. Try to minimize to deeply nested structures. Especially when the start and end of it does not fit on a screen. With subroutine and functions it is a good practice to add 'guard' code. Instead of nested if to check on conditions like: [code] If FirstCondition then If SecondCondition then if ThirdCondition then 'Do the stuff end if end if end if [/code] you can use: [code] If Firstcondition then exit sub If SecondCondition then exit sub If ThirdCondition then exit sub 'Do the stuff. [/code] This breaks the 'rule' of the function/subroutine having only one exit, but that was old style programminging and this is the better way to do it. With functions it is also good to define the returnvalue at the start of the function. Here is a small example that has all of these. It will be a function that returns a multiplied value only when both parameters are bigger then zero, otherwise the result will be zero. first the often used way a little exaggerated to make the point: [code] function Multiply(a, b) 'Multiply only when both parameters are bigger then zero if a > 0 then 'Check if A is positive if b > 0 then 'check if b is positive return a * b; end if end if 'return zero when one or both parameters are zero or negative. return 0 end function [/code] So what is 'wrong' with this? It does what it is supposed to do. First choosing the name 'Multiply' fir the function is not really good. When this is used it will confuse the reader why this is used instead of just using a *. The conditions within that function are not clear without reading that function. It is better to change the name to something more descriptive, although it sometimes can be long it is still better then a name that can be read wrong. Next it is not directly clear what type of value is returned by the function. Then you have nested checks that makes the 'body' of the function hard to find. This is the cleanest way to write functions (and subroutines) I added extra empty lines to make the 'parts' more visible. Normally you would use only one. Keeping it on one screen is best so you can read the whole function without having to scroll (also the width has to stay within one screen, a horizontal scroll is really bothersome). [code] 'returns a multiplied value only when both parameters are bigger then zero, otherwise the result will be zero. function MultiplyOnlyWhenBothBiggerThanZero(a, b) Dim ReturnValue ReturnValue = 0 if a <= 0 then return ReturnValue if b <= 0 then return ReturnValue ReturnValue = a * b return ReturnValue end function [/code] There are now 4 distinctive parts inside the function. 1. The declaration of the returnvalue. 2. The 'guards' to prevent the body of the function to be executed and returns a defined returnvalue. (It could also be used to throw errors that can be caught by the calling code) 3. The body of the function 4. The return from the function Note that the description of the function is in front of the function. In many editors you can collapse functions to just one line and that would hide the description. Any function can be made using this 'template' and when you do it consistent it will be easier to read and understand There is so much more, but i found that when you start with functions and subroutines that have clear names and a fixed style, understanding its working is so much easier and it is one step in making programs 'tidy'. Microblocks. Build with logic. |
||||
JohnS Guru ![]() Joined: 18/11/2011 Location: United KingdomPosts: 4038 |
To be fair, a percentage of programmers write terrible code no matter what language. I've seen them do it in Pascal, too. However, there are very few Pascal programs compared to the number of them written in C, so you'd expect to hear more about the C ones. It's a myth otherwise. I've met remarkably ignorant Pascal (or Modula) programmers who made outrageously wrong claims about C such as saying it makes people write unreadable code. Yeah, right, the manual points a gun at you. There are good reasons for C's success and why Pascal & Modula are at most used for teaching in comparison. However, if you want a laugh there's the IOCCC http://www.ioccc.org John |
||||
djuqa![]() Guru ![]() Joined: 23/11/2011 Location: AustraliaPosts: 447 |
I agree. I used to tutor pascal/Modula newbies at TAFE. Some shocking code snippets, but the worse I have seen was in commercial pascal code I was Hired to fix/patch/In the end totally rewrite. However I stand by my statement about C/C++. Yes there art excellent C/C++/C# programmers out there (I know, I am one of them ;}) But it was almost a rite of passage to write a C program as a "IF" statement. Or the previously mentioned C++ int Constructor overload. you can use: [code] If Firstcondition then exit sub If SecondCondition then exit sub If ThirdCondition then exit sub 'Do the stuff. [/code] This breaks the 'rule' of the function/subroutine having only one exit, but that was old style programming and this is the better way to do it. No it is not a better way of doing it. The following is a more correct method, using your logic, but still retaining the 1 exit point rule. [code] If Firstcondition then exitpoint If SecondCondition then exitpoint If ThirdCondition then exitpoint 'Do the stuff. exitpoint: exit sub [/code] Correct methods of coding never become "Old Style", hopefully they become "the Normal Method" VK4MU MicroController Units |
||||
MicroBlocks![]() Guru ![]() Joined: 12/05/2012 Location: ThailandPosts: 2209 |
Obviously everyone has their own preferences, but jumping to the end of a function/subroutines is a completely unnecessary extra step requiring the reader to find that point in the subroutine. What about the 'rule' of not using GOTO? When considering that, it is a conflict of rules! The whole point about a 'guard' is that it guards the routine from invalid conditions. It prevents 'entry'. Jumping to an exit point can introduce invalid outcomes when someone later adds code after the 'exitpoint'. It can change the working of the function because you spread around functionality. The chance that happens is real as often the one who modifies the code is not the original writer. Seen it too often, and that is why i like 'guards' the way i use threm, because it prevents that. So, my 'better' is not the same or better as your 'better'. :) And 'rules' are not absolute strict. As long as you follow rules stick to it so that you have a 'style'. The reader will then get used to the style and that adds to understanding. 'Style' is then actually more important then specific rules. Microblocks. Build with logic. |
||||
djuqa![]() Guru ![]() Joined: 23/11/2011 Location: AustraliaPosts: 447 |
When someone later adds code after the 'exitpoint' it can change the working of the function because you spread around functionality. There would/should be no code after that point. Yes there are various styles of coding. Define your style/method of coding,document it and stick to it. VK4MU MicroController Units |
||||
MicroBlocks![]() Guru ![]() Joined: 12/05/2012 Location: ThailandPosts: 2209 |
When someone later adds code after the 'exitpoint' it can change the working of the function because you spread around functionality. There would/should be no code after that point. Yes there are various styles of coding. Define your style/method of coding,document it and stick to it. You actually strengthen my argument. Your SHOULD/WOULD is not someone elses way to do things. You would need to add comment like [code] .. exitpoint: 'don't add code here! exit sub end sub [/code] to prevent someone doing just that. Microblocks. Build with logic. |
||||
djuqa![]() Guru ![]() Joined: 23/11/2011 Location: AustraliaPosts: 447 |
Exactly why I said define your style and DOCUMENT it. VK4MU MicroController Units |
||||
James_From_Canb![]() Senior Member ![]() Joined: 19/06/2011 Location: AustraliaPosts: 265 |
Sigh..... Let me step into the line of fire and describe my style. My preference for functions and subroutines is to have a single exitpoint. For anything except a very simple function I use a local variable to hold the return value. The local variable is moved to the return variable immediately before the exitpoint. The single exitpoint may mean GoTos, but in my opinion that's a lesser evil. Why the single exitpoint? 1. I can set a "value =" type breakpoints on the value of the local variable, and if the language doesn't support that I can write an IF statement and put the breakpoint on the Then statement. 2. I know where the function always exits so I can set a breakpoint there and check the local variables before they go back into the ether. Why the use of GoTos? It can be a lot easier to read than a complex IF statement, even with good indenting (says he, diving into the nearest trench for cover) Totally agree with the planning and doco, the top down programming etc. The biggest problem I used to see was people not updating their documentation at the end of a project. It was always more important to get onto the next project ASAP. Too bad for the maintenance programmers though. James My mind is aglow with whirling, transient nodes of thought careening through a cosmic vapor of invention. Hedley Lamarr, Blazing Saddles (1974) |
||||
MicroBlocks![]() Guru ![]() Joined: 12/05/2012 Location: ThailandPosts: 2209 |
I guess the concept of using a 'guard' is lost. It is normally used to capture invalid parameters and the choice for the programmer is to throw exceptions or return fixed values that can be checked by the calling code. It is part of 'defensive coding'. When functions or subroutines are reused correct parameters can not be ensured. A 'guard' ABORTS the function or subroutine. By using a guard clause in your function you actually have an ABORT and a normal EXIT point. Those are very different situations. If you combine those two conditions you obfuscate the code. Maybe this is an example that show the abort better by using ERROR. [code] 'returns a multiplied value only when both parameters are bigger then zero, otherwise the result will be zero. function MultiplyOnlyWhenBothBiggerThanZero(a, b) Dim ReturnValue ReturnValue = 0 if a <= 0 then ERROR "Invalid parameter a, must be bigger than zero!" if b <= 0 then ERROR "Invalid parameter b, must be bigger than zero!" ReturnValue = a * b return ReturnValue end function [/code] This style was chosen by a company that has more then 20 developers and 2 auditors. I am very biased of course because it was the style i developed. I can say it made my live as a programmer and auditor a lot easier when everyone followed that style and i really like that the guard code does not force me to read the rest of the code to understand its function and return values. As a bonus when used with throwing errors, developers that used someone elses routines got good helpful error messages. Microblocks. Build with logic. |
||||
bigmik![]() Guru ![]() Joined: 20/06/2011 Location: AustraliaPosts: 2949 |
Hi All, Not being a programmer (I Program with a soldering iron) My code tends to be a MAIN MENU with GOTOs and GOSUBs to required sections of code... I struggle to get my head around NUMBERLESS code (I know you use labels instead of lines but it is old age getting in the way) and I also have NEVER used DO WHILE WHEN LOOP etc but I realise these are generally preferred by the experts in the game.. I separate my blocks of code by blank lines so each `chunk' is clearly defined. I also use comments, usually at the start of each chunk and at the end of the lines of each smaller chunk within a chunk.. (See my TESTER3.BAS if interested) BASIC is wonderful (esp. MMBASIC) in that it supports several styles of programming and you are not forced down the same path like lambs... It is very hard for an oldie who did program in TRS80 basic 30+ years ago to pick up the newer routines but that is part of the fun. Anyway... just my 2c worth Regards, Mick Mick's uMite Stuff can be found >>> HERE (Kindly hosted by Dontronics) <<< |
||||
graynomad![]() Senior Member ![]() Joined: 21/07/2010 Location: AustraliaPosts: 122 |
I use both methods, multiple and single return points. For example uint32 pinSetAsAnalogInput(uint32 pin) {
uint32 retval = 0; retval |= _pinClearGpioBit (pin, GPIOREG_DIR); retval |= pinFunc (pin, FUNC_ADC); retval |= pinConfig (pin, PIN_ADMODE_ENABLED | PIN_MODE_NOPULLUP); return (retval); } This function clears a return value then that value is ORd with the return values of the three called functions and the result returned. Any non-zero value is an error. But then there's uint32 debounceCreate (uint32 ms) {
if (ms < 3 || ms > 50) { SYS_ERROR (BAD_DEBOUNCE_PERIOD); return ERROR; } /////////////////////////////////////////////////////////// ////// // Create the debounce arrays, one for each port debouncePinArrays[0] = (void*)safeMalloc(ms * 4); if (debouncePinArrays[0] == NULL) { SYS_ERROR (ERR_DEBOUNCE_INIT_FAILED | 1); return ERROR; } debouncePinArrays[1] = (void*)safeMalloc(ms * 4); if (debouncePinArrays[1] == NULL) { SYS_ERROR (ERR_DEBOUNCE_INIT_FAILED | 2); return ERROR; } /////////////////////////////////////////////////////////// ////// // Clear the arrays and set the global that indicates the debounce time memset (debouncePinArrays[0], 0, ms * 4); memset (debouncePinArrays[1], 0, ms * 4); __debounceInterval = ms; return NOERROR; } This has 4 return points, the first three return error codes and the final one a non-error code. Personally I would find that using GOTOs in the last example would not clarify the code or help with debugging, and having a set of nested IFs really sucks. Rob Gray, AKA the Graynomad, www.robgray.com |
||||
shoebuckle Senior Member ![]() Joined: 21/01/2012 Location: AustraliaPosts: 189 |
Maybe this is an example that show the abort better by using ERROR. [code] 'returns a multiplied value only when both parameters are bigger then zero, otherwise the result will be zero. function MultiplyOnlyWhenBothBiggerThanZero(a, b) Dim ReturnValue ReturnValue = 0 if a <= 0 then ERROR "Invalid parameter a, must be bigger than zero!" if b <= 0 then ERROR "Invalid parameter b, must be bigger than zero!" ReturnValue = a * b return ReturnValue end function [/code] I know I am a pedant, but for the sake of those unfamiliar with Functions, TZAdvantage's code may work fine in some other language, but it needs a couple of changes to work in MMBasic. a) Function name needs to be no more than 32 characters long b) To quote the MMBasic user guide, "To set the return value of the function you assign the value to the functions's name." So the revised code, with 4 lines to call it could be: a=1
b=3 Print MultiplyIfBothBiggerThanZero(a, b) End 'returns a multiplied value only when both parameters are bigger then zero, otherwise the result will be zero. Function MultiplyIfBothBiggerThanZero(a, b) Dim ReturnValue ReturnValue = 0 If a <= 0 Then Error "Invalid parameter a, must be bigger than zero!" If b <= 0 Then Error "Invalid parameter b, must be bigger than zero!" ReturnValue = a * b MultiplyIfBothBiggerThanZero = ReturnValue End Function Maybe I am of the old school, but I have seen enough to know that Functions and Subroutines cause less problems in a maintenance shop when they have a single entry point and a single exit, and I believe that this is still true today. Of course, in any coding "Rule", there will always be an exception which proves the rule. Cheers, Hugh |
||||
Page 1 of 2 ![]() ![]() |
![]() |
![]() |
The Back Shed's forum code is written, and hosted, in Australia. | © JAQ Software 2025 |