How can I improve/shorten my C code?

**C-UL8R** · 10-01-2020

Hi everyone,

I'm still new to C, coming from a Python background, and I was wondering if you could suggest any ways to improve/shorten my code. It's for a first-year university assignment, but I've already got it working and golfed it a bit on my own. Here it is:

Code:

#include<stdio.h>
#include <ctype.h>
int main(void) {
  char c = '\0';
  int vowelCount = 0, conCount = 0;
  while (c != '0') {
    puts("\nPlease enter a letter: ");
    scanf(" %c", &c);
    if (isalpha(c) || c == '0') {
      switch (tolower(c)) {
        case '0':
          puts("\nThank you for counting on me.");
          continue;
        case 'a':
        case 'e':
        case 'i':
        case 'o':
        case 'u':
          vowelCount++;
          for (int i = 0; i < vowelCount; i++) {
            putchar('*');
          }
          break;
        default:
          conCount++;
          for (int i = 0; i < conCount; i++) {
            putchar('!');
          }
        }
    }
    else {
      puts("\nAny character not in the English alphabet is not an accepted input for this application.\n");
    }
  }

}

Any comments welcome!

**hamster_nz** · 10-01-2020

To me, it looks pretty good.

**awsdert** · 10-01-2020

If you want to go to extremes for improving then add booleans called isVowel and isDigit, use them like this:

Code:

c = tolower(c);
isDigit = (c >= '0' & c <= '9');
isVowel = (c == 'a' | c == 'e' | c == 'i' | c == 'o' | c == 'u' );
vowelCount +=  isVowel;
if ( isDigit )
  puts("...");
else if ( !isVowel )
{
  conCount++;
  // Your loop
}

For further extremes do it in ASM and utilise the MUL instruction to set and address to goto when a statement is false to and then use the GOTO instruction I think to goto to that address or 0 (next address), 0 for continue into the "if" block, address for skip the block, it basically is a hack of sorts for removing the slowest part of code, branch instructions (used by if/else if/switch/case statements)

**laserlight** · 10-01-2020

On the other hand, if you want to be a software engineer who may one day develop software -- for other people to use and be paid for your work -- in teams with membership that can vary over time while remaining maintainable, then you would have a more balanced consideration for what constitutes "improvement" rather than having a singular focus on efficiency as the sole metric (though there was no mention of measurement) for what is good in software.

**Hodor** · 10-01-2020

Originally Posted by awsdert

If you want to go to extremes for improving then add booleans called isVowel and isDigit, use them like this:

Code:

c = tolower(c);
isDigit = (c >= '0' & c <= '9');
isVowel = (c == 'a' | c == 'e' | c == 'i' | c == 'o' | c == 'u' );
vowelCount +=  isVowel;
if ( isDigit )
  puts("...");
else if ( !isVowel )
{
  conCount++;
  // Your loop
}

For further extremes do it in ASM and utilise the MUL instruction to set and address to goto when a statement is false to and then use the GOTO instruction I think to goto to that address or 0 (next address), 0 for continue into the "if" block, address for skip the block, it basically is a hack of sorts for removing the slowest part of code, branch instructions (used by if/else if/switch/case statements)

a) Why are you using bitwise OR?
b) Parenthesis where they're not required could, arguably, be regarded as clutter
c) I have no idea what you're talking about regarding the MUL, GOTO is. Further, removing branch instructions?! What for? To remove the slowest part of the code? This is premature optimisation on steroids and would make the code an unreadable and unmaintainable mess. There is no need to avoid branch/eliminate instructions. wow. Obfuscating code by doing stuff like avoiding branches will more than likely get in the way of the compiler's optimiser and maybe (or even likely when you take things to such extremes) result in SLOWER code, not faster

**awsdert** · 10-02-2020

Originally Posted by Hodor

a) Why are you using bitwise OR?
b) Parenthesis where they're not required could, arguably, be regarded as clutter
c) I have no idea what you're talking about regarding the MUL, GOTO is. Further, removing branch instructions?! What for? To remove the slowest part of the code? This is premature optimisation on steroids and would make the code an unreadable and unmaintainable mess. There is no need to avoid branch/eliminate instructions. wow. Obfuscating code by doing stuff like avoiding branches will more than likely get in the way of the compiler's optimiser and maybe (or even likely when you take things to such extremes) result in SLOWER code, not faster

a) so the comiller doesn't try to sneak in branches for the sake of that expression
b) That's a POV thing, I prefer the brackets
c) MUL & GOTO are ASM instructions, not C language, I take it you haven't dabbled much if at all in ASM

**Hodor** · 10-02-2020

Originally Posted by awsdert

a) so the comiller doesn't try to sneak in branches for the sake of that expression
b) That's a POV thing, I prefer the brackets
c) MUL & GOTO are ASM instructions, not C language, I take it you haven't dabbled much if at all in ASM

a) Using bitwise OR the way you used it there is broken (it won't work... seriously)
b) Yep, fair enough. I and many others find them annoying
c) GOTO is not an ASM instruction. I take it that you haven't dabbled in ASM. Even if GOTO *was* an ASM instruction it's a branch

**awsdert** · 10-02-2020

Originally Posted by laserlight

On the other hand, if you want to be a software engineer who may one day develop software -- for other people to use and be paid for your work -- in teams with membership that can vary over time while remaining maintainable, then you would have a more balanced consideration for what constitutes "improvement" rather than having a singular focus on efficiency as the sole metric (though there was no mention of measurement) for what is good in software.

Part of why I said "extremes", anyways for the code I gave as long as one understands what the operations do (which any decent engineer should) then it only takes an extra second or 2 to understand what it's doing which when programming is an acceptable delay

**hamster_nz** · 10-01-2020

Oh, the origial poster might want to consider using getchar() rather than scanf() to get a character at a time.

If you do continue to use scanf() you should also be testing the return value to check that it did actually read something, and act appropriately if it didn't.

**laserlight** · 10-02-2020

Any decent engineer would point out that applying "basically is a hack of sorts for removing the slowest part of code, branch instructions" misses the forest for the trees: the slowest part of the code is the interactive I/O for getting the user to input one character at a time, but because of how the feedback is given to the user, it may be infeasible to change it without going back to the drawing board of what the program is supposed to do. Yet, this means that optimisations to try and avoid branching in the processing code likely have no observable effect at all to even the most observant of human users.

(And then for the sake of correctness rather than efficiency, the case could be made that the program should discard or treat as an input error additional characters supplied following each prompt, even though processing multiple characters at a time would be more efficient and could even make for better UX.)

**Hodor** · 10-02-2020

@awsdert Up front: I don't care if I get banned from here because of what I am about to say. I'm going to say it because I think it's important.

Stop showing code that is plainly incorrect, please. First, your use of bitwise OR (|) is dumb and incorrect. Second, even if it was correct it'd be obfuscated and that's not good. The correct expression should be using boolean OR (||).

Moving on from that brain fart you shouldn't be encouraging people to do sht like "utilise the MUL instruction to set and address to goto when a statement is false to and then use the GOTO instruction". That's just dumb and makes no sense at all. The whole sentence doesn't make sense. The OP should be writing readable code and not worrying about nonsense of the type you suggest (the OP is doing just fine).

as long as one understands what the operations do (which any decent engineer should) then it only takes an extra second or 2 to understand what it's doing which when programming is an acceptable delay

Well, I don't understand your dribble. I don't care about delays in this instance either. What do you mean my delay? The delay it takes me to parse your shtty code? You're just making stuff up. A decent engineer would write readable code and not waffle on about some bulsht weird crap that no programmer has ever suggested apart from you

**flp1969** · 10-02-2020

Originally Posted by Hodor

Stop showing code that is plainly incorrect, please. First, your use of bitwise OR (|) is dumb and incorrect. Second, even if it was correct it'd be obfuscated and that's not good. The correct expression should be using boolean OR (||).

It's not that is "incorrect", but using a bitwise OR that way he's forcing to every single expression to be evaluated, breaking the "short circuit" just to avoid some conditional branches. AND, if you take a look at optimized machine code geneted by the compiler, it makes no difference:

Code:

; Assembly created by GCC, on Linux, for x86-64 mode, with '-O2 -mtune=native' options turned on.

; int f( char a )
; { return ( a == 'a' | a == 'e' | a == 'i' | a == 'o' | a == 'u' ); }
f:
  lea ecx,[rdi-97]
  xor eax,eax
  cmp cl,20
  ja  .L1
  mov eax,1065233
  shr rax,cl
  and eax,1
.L1:
  ret

; int g( char a )
; { return ( a == 'a' || a == 'e' || a == 'i' || a == 'o' || a == 'u' ); }
g:
  lea ecx,[rdi-97]
  xor eax,eax
  cmp cl,20
  ja  .L5
  mov eax,1065233
  shr rax,cl
  and eax,1
.L5:
  ret

; int h( char a )
; { return ( a >= '9' & a <= '9' ); }
h:
  sub edi,'0'
  xor eax,eax
  cmp dil,9
  setbe al
  ret

; int i( char a )
; { return ( a >= '9' && a <= '9' ); }
i:
  sub edi,'0'
  xor eax,eax
  cmp dil,9
  setbe al
  ret

Originally Posted by Hodor

... "utilise the MUL instruction to set and address to goto when a statement is false to and then use the GOTO instruction". ... and makes no sense at all...

I agree. This makes no sense.

**awsdert** · 10-02-2020

Originally Posted by flp1969

It's not that is "incorrect", but using a bitwise OR that way he's forcing to every single expression to be evaluated, breaking the "short circuit" just to avoid some conditional branches. AND, if you take a look at optimized machine code geneted by the compiler, it makes no difference:

Code:

; Assembly created by GCC, on Linux, for x86-64 mode, with '-O2 -mtune=native' options turned on.

; int f( char a )
; { return ( a == 'a' | a == 'e' | a == 'i' | a == 'o' | a == 'u' ); }
f:
  lea ecx,[rdi-97]
  xor eax,eax
  cmp cl,20
  ja  .L1
  mov eax,1065233
  shr rax,cl
  and eax,1
.L1:
  ret

; int g( char a )
; { return ( a == 'a' || a == 'e' || a == 'i' || a == 'o' || a == 'u' ); }
g:
  lea ecx,[rdi-97]
  xor eax,eax
  cmp cl,20
  ja  .L5
  mov eax,1065233
  shr rax,cl
  and eax,1
.L5:
  ret

; int h( char a )
; { return ( a >= '9' & a <= '9' ); }
h:
  sub edi,'0'
  xor eax,eax
  cmp dil,9
  setbe al
  ret

; int i( char a )
; { return ( a >= '9' && a <= '9' ); }
i:
  sub edi,'0'
  xor eax,eax
  cmp dil,9
  setbe al
  ret

I agree. This makes no sense.

Well that was actually a helpful analysis, I didn't know the compiler still manages to recognise it as a branch-able code and interfere with code level optimizations, I'll stick to || & && then. As for the mul here's a psuedo example using made up byte code (with the assumption only 1 or 0 are the result of the comparison)

Code:

01 01 00000006 # mov %1,00000006 ; Fill register 1 with value of 6
01 01 00000006 # mov %2,00000006 ; Fill register 2 with value of 6
02 12 # cmp %1,%2 ; Compare register 1 to register 2, result will be placed in register 0
03 10 # mul  %1,%0 ; Multiply register 1 by register 0, 0 * %1 = 0, 1 * %1 = %1
04 01 # add %ac,%1 ; Add to address counter value of register 1
05 02 00000002 # add %2,00000002 ; Add to register 2 the value 2
05 02 0000001E # add %2,0000001E ; Add to register 2 30 (character '0')
06 02 # mov %0,%2 ; Fill register 0 with value of register 2
07 00000001 # syscall putchar ; print value of register 0 as next character

In this example '8' would be printed since register 1 and register 2 are equal resulting in the comparison putting value 0 into register 0 which when multiplied against register 1 results in 0 which when used by the add instruction would result in nothing being added to the address counter which results in the add 2 to register 2 being executed which then continues onto add '0' as it would've done anyway if the address counter had 6 added to it had the result of the comparison been 1, notice that at no point there does the CPU have to wait for new instructions to be loaded on the "branch" as it sees no branch to begin with, at most it just ignores the already loaded 6 bytes worth of information and just continues on with the next loaded information. Now granted this particular example does not take into account comparisons that result in anything other than 0 or 1 so would actually be quite dangerous to use without first clearing unwanted bits via the and instruction

**flp1969** · 10-02-2020

Originally Posted by awsdert

As for the mul here's a psuedo example using made up byte code (with the assumption only 1 or 0 are the result of the comparison)

Code:

01 01 00000006 # mov %1,00000006 ; Fill register 1 with value of 6
01 01 00000006 # mov %2,00000006 ; Fill register 2 with value of 6
02 12 # cmp %1,%2 ; Compare register 1 to register 2, result will be placed in register 0
03 10 # mul  %1,%0 ; Multiply register 1 by register 0, 0 * %1 = 0, 1 * %1 = %1
04 01 # add %ac,%1 ; Add to address counter value of register 1
05 02 00000002 # add %2,00000002 ; Add to register 2 the value 2
05 02 0000001E # add %2,0000001E ; Add to register 2 30 (character '0')
06 02 # mov %0,%2 ; Fill register 0 with value of register 2
07 00000001 # syscall putchar ; print value of register 0 as next character

In this example '8' would be printed since register 1 and register 2 are equal resulting in the comparison putting value 0 into register 0 which when multiplied against register 1 results in 0 which when used by the add instruction would result in nothing being added to the address counter which results in the add 2 to register 2 being executed which then continues onto add '0' as it would've done anyway if the address counter had 6 added to it had the result of the comparison been 1, notice that at no point there does the CPU have to wait for new instructions to be loaded on the "branch" as it sees no branch to begin with, at most it just ignores the already loaded 6 bytes worth of information and just continues on with the next loaded information. Now granted this particular example does not take into account comparisons that result in anything other than 0 or 1 so would actually be quite dangerous to use without first clearing unwanted bits via the and instruction

And, again, it makes no sense. What is being done with this "address counter"? Is it a "program counter" or "instruction pointer"? In that case this "add %ac,%1" is nothing else but an indirect jump and succetible to the rules of dynamic branch prediction, which is worse than conditional jumps (they follow "static rules").

You realize this MUL, in this context, is actualy an AND and this CMP is simply a subtraction?

If it is possible to avoid branches, the compiler will do it for you. Like this:

Code:

; if ( a > b ) b = a;
;
; Suppose a is in EAX and b is in EDX:
cmp eax,edx
cmovg edx,eax

No need for bogus multiplications and indirect jumps. If it is not possible the compiler will choose a less damaging code. For exemplo:

Code:

;while ( size-- ) *p++ = 0;
;
; Supose size is in ECX and p is in RDX register.
...
  jmp  .test           ; not subject of branch prediction.
.loop:
  dec ecx
  mov byte [rdx],0
  inc rdx
.test:
  test ecx,ecx
  jnz .loop          ; no penalty for jumps backwards!
...

By the way, other processors have conditional moves as well... ARM is another example.

**awsdert** · 10-02-2020

Originally Posted by Hodor

@awsdert Up front: I don't care if I get banned from here because of what I am about to say. I'm going to say it because I think it's important.

Stop showing code that is plainly incorrect, please. First, your use of bitwise OR (|) is dumb and incorrect. Second, even if it was correct it'd be obfuscated and that's not good. The correct expression should be using boolean OR (||).

Moving on from that brain fart you shouldn't be encouraging people to do sht like "utilise the MUL instruction to set and address to goto when a statement is false to and then use the GOTO instruction". That's just dumb and makes no sense at all. The whole sentence doesn't make sense. The OP should be writing readable code and not worrying about nonsense of the type you suggest (the OP is doing just fine).

Well, I don't understand your dribble. I don't care about delays in this instance either. What do you mean my delay? The delay it takes me to parse your shtty code? You're just making stuff up. A decent engineer would write readable code and not waffle on about some bulsht weird crap that no programmer has ever suggested apart from you

You understand that 0 times any number equals 0 yes?
You understand that 1 times any number equals the number that 1 was multiplied against yes?
You understand that any comparison in C only results in 1 or 0 yes?
You understand that bitwise OR just mangles the bits of 2 integers together (of which booleans are a part of) yes?
Then you should easily understand what the code is doing just by looking at it,

Thread: How can I improve/shorten my C code?

Thread Tools

Search Thread

Display

Hybrid View

How can I improve/shorten my C code?

Similar Threads

Help me shorten this code plz.

HOw can I shorten my code?

Making a function to shorten my code?

help to shorten my code.

Need to simplify/shorten this code. Help.

Tags for this Thread