Constructive criticism to my code

Greetings,
i'm working on an alternative getline (with human user input target) which would behave closer to a textbox. I'd like to see some constructive criticism and/or advices over my code in order to improve myself.
Right now it's strictly Windows only, but it will land on Linux too someday.

Link to github repository:
https://github.com/Sephirothbahamut/AdvancedGetline
(i tried to document it a bit in the readme)

thanks in advance,
Barnack
delete something; should be delete [] something;

getline() seems particularly complex to me, it ought to be refactored in my view.

#define's should happen after #include unless you want to set something in one of those include files.
Last edited on
thanks for the define advice and for making me notice i forgot the "[]" part.
About getline(), my main concern is how much it will change once i add Linux supprt, having some different escapes, a totally different way to define colors. Maybe i'd just do better encircling the whole getline() function with the OS-specific #ifdef guard and make two totally separate versions of it
Regarding getline, there's so much inline code that the algorithm is lost, and you just need a parenthesis at the wrong level somewhere to break it.

You could use handlers for the switch cases to begin with. You'll be surprised how readable it becomes, and along with that comes maintainability, and you ability review what you've done.
Regarding getline, there's so much inline code that the algorithm is lost, and you just need a parenthesis at the wrong level somewhere to break it.

Well there's ide's syntax highlighting for that ahahah.

What do you mean by "handlers for the switch cases"? I'm really interested...
What do you mean by "handlers for the switch cases"? I'm really interested...
For example:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
char* getline()
{
	// ..
	do {
        ch = getch();
	switch (ch) {
	case '\b':
		ret = onBackspace(); break;

	case 'q':case 'w':case 'e':case 'r':case 't':case 'y':case 'u':case 'i'://chars
	case 'o':case 'p':case 'a':case 's':case 'd':case 'f':case 'g':case 'h':
	case 'j':case 'k':case 'l':case 'z':case 'x':case 'c':case 'v':case 'b':
	case 'n':case 'm':case 'Q':case 'W':case 'E':case 'R':case 'T':case 'Y':
	case 'U':case 'I':case 'O':case 'P':case 'A':case 'S':case 'D':case 'F':
	case 'G':case 'H':case 'J':case 'K':case 'L':case 'Z':case 'X':case 'C':
	case 'V':case 'B':case 'N':case 'M':
	case '0':case '1':case '2':case '3':case '4':case '5':case '6':case '7'://numbers
	case '8':case '9':
	case '(':case ')':case '[':case ']':case '{':case '}':case '<':case '>'://parenthesis
	case '.':case ',':case ':':case ';':case '!':case '?':case '\'':case '"'://punctuation
	case '/':case '*':case '-':case '+':case '%':case '^':case '='://operations
	case '\\':case '|':case '&':case '$':case '_':case '~':case ' '://other
		ret = onChar();
		break;

	// ...
        }
    }
    while ();.
}


Obviously, it can be evolved further. Also, consider using smart pointers, in this case, getline() could return a unique_ptr<char>.
Last edited on
oh you mean a function. i never called a function "handler"... handlers usually are things you use to interact with other things
That's exactly what these are.
Topic archived. No new replies allowed.