-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Code style guidelines #1152
Comments
My problem right now is that I wanted to work on extracting functions like |
Whether enforced by the language or not I prefer seeing the declaration of a thing before it's first usage. But in the end I think what matters most is that related functions are close to each other in the code.
Both have valid use cases. But most of the time I think declaring structs separately is the way to go, because it is better when having multiple structs in one file.
They are fine where they are in my opinion. Just like tools they conceptually belong to items.
Importing root is the preferred way, but it sometimes breaks ZLS, and until that's more stable I'd like to leave it for now.
Another tooling issue, until ZLS has a way to automatically create and destroy aliases it does not make sense to enforce it.
Fields should always be at the beginning of the structs, unless there is something that conceptually should be declared up top (e.g. import statement or sometimes other declarations) |
In Cubyz, files are not documented with top level comment. As a result, you don't have a clear statement describing what given file actually contains. You are saying that they conceptually belong to items, so please, make an thought experiment and try to formulate said top level comment, something along the lines of Now when I try to do that, what I get is something along the lines of What we could do is to group them under
This means that each file has clearly stated purpose. Removes name collisions, as |
I will think about this. |
There is a clear description: The file name. I agree with you that it would be nice if every file only had one nice and simple responsibility (like e.g. commands or materials), but in my opinion that's not always useful. The reason why it works so well for commands is, because each command has a clear purpose, and commands don't interact with each other, so if I implement a new feature for a command I really only need to touch that one file. On the other hand Tools, BaseItems and Materials do interact quite a lot. And it's nice to work on them without having to switch between files all the time. And VSCode's minimap makes navigating even large files really nice in my opinion. Much nicer than using file tabs.
The same thing could be done with regular namespaces, it could easily be
I think that most of the proposed name changes are neutral (it neither makes typing nor reading easier/harder), this one would even make things worse in my opinion, unless you want a separate file just for tool types.
I don't think "assets" is a good way to group things. All of these things do completely different things. |
Single word is not a description, it is a symbol - its open for interpretation. I agree that comments are a burden and there is no room for them when project is in pre-first release stage. However, I do not agree that one should not pay attention to them, especially when they are done correctly, see numpy.fft or cPython json module. They exist to provide documentation of features in the module, a guide. This is very useful if module is stable, since it makes it so documentation is actually close to code compared to having separate markdown files.
Whether you will have to touch multiple files or not depends heavily on what you are doing. If you decide to change something in how commands work in general, by adding new field or whatever, you will have to touch command files too. It's only a matter of how hard requirements change. It is not possible to predict how requirements will change, so don't try too. Do what's best now, as often a best way to prepare for future complexity is to keep what here today as simple as possible. Why don't we just place everything in one file (except of commands since they are so independent)? Maybe, just maybe because setting boundaries keeps complexity of dependencies manageable. If we keep stuff together and have direct access to everything, then everything is a dependency for us. Complexity grows exponentially.
I don't see a particular difference (between switching and scrolling). The problem is that the second you have to think where to look, do actual searching, you are wasting time. I'm a bit digressing here, but hear me out: If we follow a rule that implementation details are always below, then we can always scroll below current code an often we will find what we are looking for. This is a branch prediction hit, it is good for efficiency. The better our branch predictions are the, the less often we have go back to actually searching for things. Regardless, if we are mentioning VSC features, you can also use You neither need scroll nor tabs to navigate codebase.
This was not a detailed refactor proposal, more of a expression of a general idea. What I noticed is that you are placing a lot of functionality in one file without setting boundaries. I have seen that before at corporate scale, trust me, you would not want to navigate a 30+k lines of implementation Vulkan Buffer structure in C++. More stuff you have together the faster it will accumulate new changes growing out of control.
This is not true. If it was true, namespaces would not overtake C style naming conventions in modern programming languages. If there is no name collision, renaming
Possible, naming things is hard.
Naming things is hard, structuring is hard too. No matter what principles you chose, requirements may change. Does that mean we are not supposed to structure code at all? There is an antipattern describing this approach, it has been proven destructive. I am not emotionally tied to grouping those files in a way I have shown, neither I care about the names, as long as the code is as efficient to traverse, read and change as possible. |
It's just enough to tell me what it is about.
These are great example for comments that I would skip right over. "Just show me some code already"
Yes, I absolutely agree. However Cubyz source code is neither stable, nor intended to be used as a module in third-party projects.
Of course it does, but for commands in particular I'd only touch one file most of the time, in the rare case of a change in its interface I'd touch all files at once. In graph theory you'd call this a tree, if you change the node, you need to change the children, but there are no links between the children. This is obviously different to the many dependencies inside the items.zig file.
I don't think we need to talk about this. Let's try to avoid the straw man fallacy here.
I do set boundaries, and I do regular split files when they get too large, or collect too many different things.
Doesn't matter for functions. Most functions are called on a separate line, and whether the function name is 5 characters longer or not makes no difference to me.
Those are not really words that you read. Reading a 250 word essay takes time and effort, reading 250 words of redundant variable names just get filtered out by my brain.
Another straw man, I never said that, and the code in question isn't even unstructured to begin with in my opinion. |
This is "I came here to understand, own and (possibly) modify" mindset. You can't understand, own and modify everything, it's not a sustainable model. You can try going through numpy's code to understand how to use function X and all of the quirks of it's 6 parameters, but you will not capitalize on that knowledge since you had to only use it once. It would be much more efficient to read summary for that one argument you care about and forget. I'll stop here, since we just clearly have different view on how code should be structured and that's okay. After all it is not my burden to maintain Cubyz, I can just read once and forget and do useful stuff anyway I guess if I am wrong then it does not matter, and if I am right, you will maybe see for you self. I think we should add what we established to the contribution guidelines so this it is clear that there is some opinion about that matter. This should be beneficial for future contributors to align better with your code style. |
This issue is intended to aggregate discussions about code style practices in Cubyz codebase.
As of creation time of this issue, contribution guidelines state that:
Style practices to be discussed:
Zig source files are implicitly structs, with a name equal to the file's basename with the extension truncated. @import returns the struct type corresponding to the file.
. Therefore unless creating generic data structure, creation of simple struct can be tackled by placing fields directly in file. This has a side effect of promoting one struct per file and hence separation of functionality into dedicated files and removes redundant namespaces. On the other hand, this creates inconsistency with generic structs which have to be created as functions. Currently we can find all possible mixtures of structs, generics, enums and unions in codebase, hence making codebase inconsistent and making it hard to judge which notation to chose, since in many cases they are equivalent.@import
builtin function which can be used to add files to build and access field and declarations inside them if declared public. Currently we are multiple different practices regarding importing elements from different modules. For example, importingsrc/blocks.zig
is done sometimes byconst blocks = @import("blocks.zig");
while in other casesconst main = @import("root");const blocks = main.blocks;
. There are technical reasons to use second form, but first once could be consistently replaced with second one in most cases.src/items.zig
, structBlock
is aliased while importedconst blocks = @import("blocks.zig"); const Block = blocks.Block;
but thenmain.blocks.Block
appears in code anyway. A rule could be to always alias if accessing object through two or more namespaces (main.blocks.
) or when using object at least 3 times. This will also incentivize using more descriptive names to avoid collisions with module names.The text was updated successfully, but these errors were encountered: