Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

Add more styling options to class wizard

<< < (2/4) > >>

oBFusCATed:
I don't think you need to introduce the usage of the function in the whole plugin, just the new places. But this is up to you, you're reading and modifying this code. I doubt that I'd touch it for anything other but reviewing patches.  8)

oBFusCATed:
1. Do not use virtual and override at the same time
2. Do not write code like for (auto x: container) prefer for (const MyType &x: container) there is no point hiding the type. I don't understand why they teach people to do it. Using just auto would do also a copy and I doubt you want to do it.
3. Do not directly cast an int to enum without checking if the range matches.
4. Do not prefix enum types with 'e'.
5. Write more comments what variables and functions do.

I guess you can push/commit this whenever you like...

sodev:

--- Quote from: oBFusCATed on March 12, 2019, 11:40:11 am ---2. Do not write code like for (auto x: container) prefer for (const MyType &x: container) there is no point hiding the type. I don't understand why they teach people to do it. Using just auto would do also a copy and I doubt you want to do it.

--- End quote ---

Do write for (const auto& x : container) if you dont modify the items, do write for (auto& x : container) if you do modify the items, do write for (auto&& x : container) if you intent to move the items away (or you want/need to be flexible in using case 1 or 2 and can live with the fact if after refactoring your container returns copies instead references that your code happily modifies the copies instead of the items undetected).

You use auto bascially for 4 reasons:

* Easier Refactoring: Less changes of places that stay syntactically the same, you dont need to change the places that define stuff or only pass it around, only the places that do something with the stuff directly
* Prevent unintented type conversions: unsigned int? size_t? container::size_type?
* Define types only on one side: std::shared_ptr<std::string> ptr = std::make_shared<std::string>() vs auto ptr = std::make_shared<std::string>()
* Less Writing: std::map<std::string, std::set<std::pair<std::shared_ptr<MyFancyClass>, unsigned int>>>::iterator item = container.find("foo") vs. auto item = container.find("foo") :D

oBFusCATed:
@sodev:
1. this is a stupid reason, how do you refeature (there is no such thing as refactoring) a code you cannot understand because all the type information is hidden from you behind auto?
2. this rarely happens and it not a real reason and auto of course is quite confusing if you don't know the actual rules.
3. this is sort of valid
4. this leads to unreadable code for me - it is far better to use typedefs for the container and ::iterator.

sodev:
@oBFusCATed
I don't want to derail this topic too much, this can lead to philosophical questions like tabs vs. space, but these range-based for loops are one of the best places to use auto imho, so i couldn't resist.

The key point is to write flexible, reusable code and to achieve that you don't "code against the type" but "code against the properties", and for this you don't need the concrete type. For example pointers, in most locations it doesn't matter if it's a raw pointer or a smart pointer (it only matters if you need to create or destroy it), you only need to know what you can do with it, like dereference.

Another example that touches point 1-3 is if you need to store the return value of a method. Something like that:


--- Code: ---unsigned int index = getIndex(const Item& item);
--- End code ---

Somewhere in your code this method is defined and in that place its return type is defined. Now in this line you again define its return type. You define the same thing in two places, bad if you need to change the definition of the method someday because you need to change all the others places as well. And does it really matter to see in that line that index is an unsigned int? What if it is a size_t? uint64_t? std::vector<Item>::size_type? It is an index, you can increment it, decrement it, compare it against some capacity value, thats what you need to know.

Actually this simple line is one of the reasons why i can't port a quite sized codebase to run in 32 Bit or 64 Bit, the method returns a size_type which changes from 32 Bit to 64 Bit but the unsigned int stays, depending on platform, 32 Bit. With auto i wouldn't need to take care of these places.

Navigation

[0] Message Index

[#] Next page

[*] Previous page