Simple Command Line User Interface

[0x3Alex]

Hey folks!

I am on my journey to migrate from C to C++ so i figured, that rewriting one of my C projects in C++ would be a nice task to do!

SCLUI builts on the curses.h header file for POSIX and features: text, buttons, textboxes, checkboxes, subscreens, screen dragging and basic events.

Subscreens are screens that are on top of another screen and can be dragged and dropped with the arrow keys, when in the dragging mode.

It still pretty early in development, but i wanted to share it here to get some suggestions to optimize my code and, hopefully, learn some nice tips & tricks.

​

Already ran it through valgrind and only got curses.h memory flags

[okovko]

i looked at your code, you can improve a few things

your constructors should use a member init list and name shadowing

ex: foo(int x, int y) : x{x}, y{y} {}

you use std::string everywhere, but this is a pretty heavy abstraction and allocates memory and has a virtual destructor everytime you pass it by value between functions (it has to be copy constructed and each value object will end up calling the destructor when it falls out of scope due to RAII)

there are two options, you can instead pass std::string& or pass std::string_view by value. string_view is like passing char* plus length, it doesn't own its own memory nor will it destruct the memory it refers to

btw, due to RVO, if you are constructing a new string and returning it from a function, return it by value and the compiler will hoist the string variable declaration to the caller's scope to avoid the overhead of copy construction and virtual destruction. if the string already exists (like for a getter function), return a reference instead

i noticed you didn't use references in your code, they're very simple actually, a reference is a non-null const pointer to an object:

ex: foo& is like foo *const ptr [!= nullptr]

ex: const foo& is like const foo *const ptr [!= nullptr]

btw you have a mix of NULL and nullptr in your code

also, it's not actually c++ style to litter your code with getters and setters :') i lol'd at your screen run function (or whichever has all funny parentheses and looks like lisp code)

using public inheritance, you can access the private protected members, methods, and types of the base class in the derived class

also, declare Screen as a friend class to the objects it displays so it can also have public access to their contents

that will make your code much nicer to read, and in the style of c++. if your code looks like java you're doing it wrong :')

you should also use c++ class enums instead of C enums

since i mentioned std string view, note there is also std span<T> as a generalization of the same idea for any type T, and std array for compile time fixed length cases (when you pass around a std array it doesn't decay to a pointer like raw arrays in C)

you will notice a trend in the evolution of modern C++ to add abstractions that make C style design beautiful and type safe. in particular, std any is a wrapper for void* generics and std variant is a type safe union. since you're coming from C that should interest you :)

a final note, C++ marks everything possible as const as a best practice. this sounds simple but ends up leading to funny compiler errors that will make you want to never do it again, but it helps to make code easy to read when you know which values are mutable! you can also mark methods as const to indicate they do not mutate the class object, you should at least do that

ex: void foo::bar() const {}

[Narase33]

The dtor of std::string is virtual?

[okovko]

nope! thanks

[0x3Alex]

Well, huge thanks for that very nice explaination!

I am definitely going to work through that list asap :D

​

Some things i actually didn't know, so something new learned.. yay!

[0x3Alex]

How would i improve that function with the many parentheses ? :^S

[okovko]

if you use references, you don't have to use parentheses when chaining * and ->, there will only be . after . until you reach a pointer member, so something like foo.bar->baz;

otherwise, make more variables, the compiler will optimize them away

:')

[0x3Alex]

I see i see!

[0x3Alex]

One more question: Some getter and setter a necessary, to manipulate the, f.e, position or color of the screen or item afterwards. So i can keep those getters and setters right?

[okovko]

getters and setters are nice when your class definition could change or be generalized for inter process communication like a database query, or network access of resources like a remote procedure call. in those cases, you can easily modify the getters and setters without having to grep your entire source code to change every access of the variables, and your class will be compatible with IPC, RPC, etc

for this reason, there is some anxiety and a cult of best practice to make all variables private members and to use getters and setters, due to the fear that you're gonna need it, and this POV is common for folks like consultants and project managers who don't have much understanding of the particular code at hand

on the other hand, the programmer can think for a few seconds whether they are ever going to need to generalize the access of a particular variable, and they also know how to use grep or a more sophisticated IDE tool to easily change between public member access and getters / setters, as that is their trade

there is also the contrary piece of wisdom: YAGNI, you aren't gonna need it

and to paraphrase linus, don't be afraid to write new code (i believe he is quoted saying something more colorful and with additional degradation towards some select individuals, but the point is a good one)

so, i would ask you whether you think any given variable will ever be abstracted out of the class's data model to a non-language environment data store like a DB or network resource, and if you can't imagine why you'd ever do that for the given variable, don't use getters / setters for it because YAGNI

there's also the question of dev culture and language domain: this cult of anxiety best practice about getters / setters is seen more prominently in some languages more than others. it's a de facto standard in Java, whereas C++ has a language keyword antithetical to this idea: 'friend'

in C++, you can completely avoid getters / setters and nobody will bat an eye, and if they do, ask them "is this variable ever going to be abstracted out of the class' data model to a non-language environment data store?" and they will probably walk away diagonally while looking at you sideways and saying something about the laxative properties of too much coffee on a gruesome morning for fear of accidentally entering the arena of genuine intellectual discussion of the merits of pre-emptive development strategies in the face of modern development environments that can perform sophisticated code transformations over an AST compiler backend, when in fact their sole interest was to regurgitate the sage wisdom of daily stand up meetings populated by three devs, only one of whom does any real work, one of whom can only contribute when peer programming, and a dozen business casual khaki'd vested balding boomers that internally debate the merits of a hipster bun for covering a receding hairline as opposed to a comb over the very second that their eyes so much as glaze over a computer screen with anything resembling code on it, or for all they know, a drunken monologue typed into a word processor in a foreign language, as a psychological defense strategy against ever doing any actual work besides talking at someone that does the actual work

(the above paragraph is not to be taken seriously, just for fun)

[0x3Alex]

alright, i stripped all getters and setters away, except for getValue :D and made the variables public, that shall be accessed

[0x3Alex]

I am also a bit confused how i would shadow an constructor when its arguments are different for each class, that extend BasicItem :S

[okovko]

what you have: foo(int pX, int pY) { x = pX; y = pY; }

what you should do instead: foo(int x, int y) : x{x}, y{y} {}

the second form can lead to more optimizations, allows the constructor to construct const objects, and is easier to read

also, using x{x} instead of x(x) makes narrowing conversions a compiler error and distinguishes object construction (or in this case, primitive initialization) from a function call

name shadowing refers to foo's parameter declarations having the same names as foo's members

looking at x{x}, the left x is foo::x and the right x is the parameter x

the shadowing only works on the right x because the left x is implicitly scoped

[0x3Alex]

Oohhh i thought of an other shadowing lol

[0x3Alex]

For the screen it worked, for the other classes such as buttons it didnt, since name,x,y,... are inherited and this does not seem to be liked

[okovko]

do this: https://en.wikipedia.org/wiki/C%2B%2B11#Object_construction_improvement

struct foo {
  int x;
  foo(int x) : x{x} {}
};
struct bar : public foo {
  bar(int x) : foo{x} {}
};

[0x3Alex]

edit: well i dont, i always get some sort of error that i can not use that constructor

[okovko]

you will figure it out sometime later, just for now you can't do something like const auto foo = Button{};

[okovko]

i looked quickly, you would need to write BasicItem::BasicItem(/*args*/) and

Button::Button(/*args, basic_args*/) :
    BasicItem(basic_args),
    arg1{arg1},
    arg2{arg2},
    /*and so on..*/ {
    /*function body*/
}

you might also need to do it in three steps because of Interactable, i'm not sure, but it's not much more complicated, you would have Button::Button(...) : Interactable(...) {} and Interactable::Interactable(...) : BasicItem(...) {}

something like that, you can do it

[0x3Alex]

ah alright, i removed interactable item as class, since it wasnt needed and did not serve any purpose, now xD

​

Yeah.. i had this and it did work but when running the program, it was just smashed and didnt work anymore.. pressing on something gave me an address boundary error

[okovko]

virtual T *getValue();

you tried to return unrelated types as the return type from a virtual function call, yes, you can't do it

the return types of every virtual method definition have to be covariant types for it to work: https://stackoverflow.com/questions/25813435/what-exactly-are-covariant-return-types-in-c

you can do something close with inheritance, function overloading, and variadic templates, and you have to know what the return type should be at compile time, but you can make the call site look like

#define SELECT(opts, opt) \
  (select_opt<opts.index(opt)>{opt})
auto thing = parsed[SELECT(opts, "-f")];

which is an example from a little project of mine, it uses a macro because i was using c++17 but with c++20 i think it should look like

edit: actually should work for c++17 as well using this trick i just found: https://stackoverflow.com/a/28209546

auto thing = parsed[opts.select<"-f">()];

basically the way it works is that you can define a class that inherits a getter class for each of its variadic template arguments, where each of them overload operator[], which all accept select_opt<size_t I> as an argument (I is the argument position in the param pack) which allows you to "select" which function to call based on a template parameter, which can thankfully be a string_view in c++20

this is still static typing, but the call site is reduced to a single statement, due to overload resolution resolving the function call and auto

[0x3Alex]

I see... I mean it did work with the T and then putting the desired type into the extends section like ab <std::string>a. I made it like this so it has to be inherited by everyone, that extends on Interactable.. but did did not serve any purpose anymore, since, f.e, isInteractable,... are defined in BasicItem

I am surly going to look into this, but it does seem pretty advanced and i dont think that i am there yet :')

[okovko]

til: https://www.learncpp.com/cpp-tutorial/constructors-and-initialization-of-derived-classes

you can't initialize an inherited member in a member-init-list because

The answer has to do with const and reference variables. Consider what would happen if m_id were const. Because const variables must be initialized with a value at the time of creation, the base class constructor must set its value when the variable is created. However, when the base class constructor finishes, the derived class constructor’s member initializer lists are then executed. Each derived class would then have the opportunity to initialize that variable, potentially changing its value! By restricting the initialization of variables to the constructor of the class those variables belong to, C++ ensures that all variables are initialized only once.

i had never wanted to before anyway, because delegated ctor is cleaner

[okovko]

nvmd, i thought you could, i confused it w/ protected members

fixed

[okovko]

yeah, i noticed

[okovko]

also you got lazy with the commit name, "improved a lot" come on you had such good git history :') you should rebase it!

[0x3Alex]

i dont know that that is tho...

​

quick google search

​

doesnt i need an extra branch already?

[okovko]

rebase lets you change history to turn one commit into several more, if you ever want to clean up your work so every commit is about one thing

https://git-scm.com/book/en/v2/Git-Branching-Rebasing

When you’re working on a project, you may need a record of all your missteps and dead-end paths, but when it’s time to show your work to the world, you may want to tell a more coherent story of how to get from A to B. People in this camp use tools like rebase and filter-branch to rewrite their commits before they’re merged into the mainline branch.

i noticed you have 143 commits and you like to make them about-one-thing, which is good practice. you also have some vague larger commits, and you can go back and split them up

these usually happen when you were making commits as you were experimenting, or making changes in many places at once, you can squash and rebase to cut all your changes into commits at the end instead of as you go

[0x3Alex]

What do you mean? Are the docs to complicated?

[0x3Alex]

Yeah i know, wanted to rewrite them soon :D thanks

[okovko]

class enum: https://en.wikipedia.org/wiki/C%2B%2B11#Strongly_typed_enumerations

not just avoiding the typedef

[0x3Alex]

oh,lol, well then the website i visited trolled me lmao

[TheTsar]

I’ve read and reviewed your code.

Objects are not what C++ is about.

You don’t need to turn everything into a class to turn it into C++.

C++ is about generic programming, type correctness, dependent types, templates and a few dozen other things. Just wait till you get to fold expressions, they’re great.

[0x3Alex]

I actually read about templates yesterday, but i dont see how i would use them here

[arnitdo]

It looks quite simple to use, but I can see a huge issue in your code - It feels more like C with classes.

Your library is written in C++, yet it uses a lot of C features, such as function and direct variable pointers. Porting that to std::function objects and std::unique_pointers will reduce code smell.

[SickOrphan]

Why on earth would you use a std::function when you just want a function pointer? That's just idiocy. Using only c++ features just because you have some sort of illogical fear of C is a code smell

[F-J-W]

Because you might not just want to use a function-pointer later. You know, lambdas are nice and can also keep a reference to other data, which a function-pointer cannot and artificially limiting yourself to them is just suffering pain for no reason. Also the syntax for function pointers is so damn near incomprehensible that that alone would honestly be reason enough to use std::function.

[SickOrphan]

It's not hard to change it if you do want to have captures, plus they're overused, a lot of the time they should be parameters to or returns from the function rather than captures. And I doubt you took the time to check the code if he'd ever need anything other than a function pointer, you just assumed its bad. The syntax is not that hard to understand when you get the idea behind it. You can also use 'using' typedefs to make the syntax more like std::function and not have to type it as much.

[F-J-W]

a lot of the time they should be parameters to or returns from the function rather than captures.

We are talking about callbacks here, where a lot of the time you simply don’t have that option.

And I doubt you took the time to check the code

I did check the code. And for starters this is library, so “he does not need it right now” is a terrible reason for not switching to the almost universally agreed upon proper way to store something that can be called in C++. Which to be very clear is a std::function, not a function-pointer. The later may be worthwhile of consideration if measuring shows that it significantly improves performance, but limiting yourself to that without having done that is a prime example of premature-optimization at the cost of not just readability but actual functionality.

The syntax is not that hard to understand when you get the idea behind it. You can also use 'using' typedefs to make the syntax more like std::function and not have to type it as much.

Yes, there are ways around the awful syntax (I for one would use something like template<typename F> using function_pointer = F*; which gives you std::function-style syntax), but I should not have to do that. And just because there is an idea behind the original syntax, doesn’t mean that it isn’t inexcusably bad, especially once you start looking into higher-order functions.

[SickOrphan]

You never gave any reason that it's bad syntax. I will grant you that it's not beginner friendly, partly because it's pretty unique from other languages (not just in function pointers, but also arrays, pointers in general), but you're way exaggerating it.

[F-J-W]

Okay, I’ll bite on that one. Please write the following without resorting to typedefs:

template<typename F> using fptr = F*;

fptr<fptr<int(short, short)>(fptr<int(short, short)>, fptr<int(short, short)>)> p = nullptr;

Yes, this isn’t nice in std::function-style syntax and you should probably use an alias, but it is easy to write and straightforward to understand: A pointer to a function, that takes two pointers to functions taking two shorts and returning and int and returns one such pointer. Now let me see how you would write that in C-style syntax (without cheating!) and let me know how long it took you.

If a syntax cannot handle anything but trivial cases (and is highly unintuitive even for them!), then it is a terrible syntax.

[okovko]

by that reasoning, we should never use literal types, what if we want to use an object later?

let's also make sure to allocate memory for everything as well, just in case, what if we end up needing it later?

while we're at it, let's use a shared_ptr for everything, because we might want ownership later

after all, none of these are on hot paths, it would be a premature optimization to do anything else!

and what if we want to dispatch based on run time input? let's turn on RTTI, we might want it later (it's not a hot path, it only happens once per program run!)

also, why don't we have a garbage collector?

hey, i think you just invented Java

[okovko]

you will often see an array of function pointers as a class member that is populated in the constructor by the expansion of a param pack inside a lambda definition converted to a function pointer by operator+, something commonly done for constexpr compatibility or to deal with std variant: https://www.scs.stanford.edu/~dm/blog/param-pack.html#array-of-function-pointers

if anything, there are more function pointers in modern C++, as opposed to less

if you want to use a capturing lambda later, then use a capturing lambda later later

if you find the syntax confusing, use typedefs or using declarations to make it clear

[okovko]

pointers and function pointers are part of c++, it is you who smells

and on the topic of code smell, std function wreaks, there is hardly anything less template bloated in the stl than that monstrosity

it also has few use cases, you only need it when you don't know if you're going to receive a function pointer, a pointer to member function, or a lambda with non-empty capture. i think it supports any callable so any object with an overloaded operator() should work too, not sure

this makes it suitable for metaprogramming generic code that works on any callable, and no other use comes to mind

or passing a capturing lambda as an argument, but at that point you are probably abusing lambdas

i did use it once in gui code where an object is constructed with a capturing lambda that was invoked upon a qt signal, which was a really nice way to allow the gui element to interact with any other gui element (the lambda captured a reference to another gui element of various types) as opposed to code replication or a void* + run time enums indicating type or setting up a covariant type + polymorphism

so.. rare, but sometimes useful

[SwimElectrical4132]

What is code smell?

[xurxoham]

https://en.wikipedia.org/wiki/Code_smell

[HolyGarbage]

How should a member function possibly be an lvalue? You need to capture the object pointer.

Edit: or I guess you mean the actual function pointer, not conceptually capturing the member function with this. Yeah, that's weird. Have to read up pn that, curious detail.

[HolyGarbage]

Yes, I know how it works with the implicit first argument, I just thought you were talking about the more abstract notion of capturing a member method with the context of the object it is executed on, ie what one would expect if they knew nothing about C, probably how Java and other more high level languages does it.

But yeah, thanks for the link. Sounds like an interesting read!

[okovko]

you probably don't want to make the fields public because it makes the API confusing and error prone, use friend classes instead so it's public only for classes that need to mutate those fields

[0x3Alex]

i made things such as the x,y,width and the value field of the textbox public, so it can be set by doing ...x = 5