There's a ton of important work to be done towards modularization, for example eliminating the global sketch object. But I don't really see how moving class definitions around helps anything...

@whitequark
let it be a first step

It's the first step towards that. Once the headers are all separated, you can see where the circular dependencies are. For example, if we wanted to extract out our NURBS functionality into our own library, we'd have to take out the references to Group. Splitting the headers out lets you see those dependencies, and will make it easier to split the functionality up in the future. I didn't want to do any changes to the actual .cpp files yet; I just wanted to get the ball rolling and make those future changes easier.

@jwesthues What do you think?

I see how this change would make the code seem easier to understand. I'm less convinced it would actually make the code easier to understand.

@marthinwurer I might suggest that you make your first change to SolveSpace (or other free software projects) an isolated feature or bugfix within the style and structure of the project as it currently exists, rather than a big change to style and structure. @whitequark may have some suggestions, or there's the issue tracker.

A small change is easier for a maintainer to digest, and it's also a good step towards more deeply understanding the codebase. This will help make subsequent bigger changes. Alternatively, you could start by drafting documentation of the current class structure, not just what's obvious from the headers but the way the objects are actually used and interact. This would be helpful both to yourself and to future contributors, and whitequark and others can help or correct where you're unsure. Sorry for not responding earlier on the forums.

Thanks, you have expressed my feelings also well.

So there is no any way to change things like this? Please describe how this can be changed, because this is really problematic thing for new contributors because modularization can simplify source code understanding and navigation.

@Evil-Spirit In the past, I have first modularized code, and then plucked it out into different fields. For example, that's how I've done the render/ and platform/. That's the way it should be done IMO.

Also, I'm not sure if "one file per class" rule is useful. This isn't Java; being able to get an overview of an entire component's interface, grouped in meaningful blocks, is valuable. (And there's nothing wrong with free functions either.)

@jwesthues That makes sense. Would #121 be a reasonable feature to start with?

@whitequark but why you don't merge my changes which makes sketch non-global? I've made it a long time ago.

but why you don't merge my changes which makes sketch non-global? I've made it a long time ago.

There is no reason other than my health and general lack of time; I absolutely want to see those changes merged. I do give SolveSpace attention, but I also have to pay my bills...

That makes sense. Would #121 be a reasonable feature to start with?

That would be a fairly sizable change. If you'd like to take it up, please discuss it with me first; we'd need to work out a plan to migrate tools one by one from the old system to the new one that would allow handling tool workflows in a more structured way.

Ok, would #301 be better? If not, do you have any suggestions for a ticket to start on?

@marthinwurer #301 seems good, and useful.

Ok, I'll start on that this weekend when I get time.

Ok, just got back into development. Some classes 100% need to be broken out into their own files for ease of navigation. Having SolveSpaceUI and Sketch in the same header makes using a C++ IDE (I use CLion) to try to navigate the codebase painful, so I'll be breaking those out into separate headers no matter what. As a new developer to this project, I have no clue where any code components are, and because there's little documentation, it's hard to tell what I need to change or what things do without reading all the code, which is hard when everything is in one massive header. I'll also be setting up the basics for Doxygen, which will let us easily build docs for the codebase javadoc-style. Also, is there a reason why operator overloading (mainly []) is not used for the list classes besides historical reasons?

As a new developer to this project, I have no clue where any code components are, and because there's little documentation, it's hard to tell what I need to change or what things do without reading all the code, which is hard when everything is in one massive header.

Please consider that after you move all of the headers around, it will be even harder to understand, because right now you can use git blame to see what commit introduced the code and why, but after that all the lines will just be attributed to you. This is the reason I haven't refactored anything "just to make the code slightly nicer" so far, and stuck to refactoring where it gives a specific, nontrivial benefit--without being able to consult change history I would have gotten nowhere with this project.

Also, is there a reason why operator overloading (mainly []) is not used for the list classes besides historical reasons?

Operator overloading, like STL and many other C++ features such as class member visibility, were not used anywhere for historical reasons. I am introducing some of them into the codebase (STL is now used where it makes good sense), but not some others (C++ implements visibility completely wrong and it should preferably not be used anywhere). Adding specifically operator overloading would be a massive, multiple month change that brings absolutely no specific benefits but adds bugs.

Please consider that after you move all of the headers around, it will be even harder to understand, because right now you can use git blame to see what commit introduced the code and why, but after that all the lines will just be attributed to you. This is the reason I haven't refactored anything "just to make the code slightly nicer" so far, and stuck to refactoring where it gives a specific, nontrivial benefit--without being able to consult change history I would have gotten nowhere with this project.

Yeah, you're right, I hadn't considered that aspect.

More questions: Why does the compile by itself, and then be ingnored in the main build? Why doesn't the main program just use the library instead of doing all the fun conditional compilation stuff?

Yeah, you're right, I hadn't considered that aspect.

Anyway, I'd be really happy to eventually get SolveSpace in a state where the sources are somewhat more structured, but I hope you can see why other things, like making code actually modular (and not just easier to navigate) were higher priority.

Why doesn't the main program just use the library instead of doing all the fun conditional compilation stuff?

Because the geometric core contains a lot of UI-related code. E.g. compare Entity with EntityBase. All this UI-related code has to be removed in the library build (so that it doesn't drag in any desktop dependencies), but everything was written with a GUI frontend in mind, so it's not really structured in a way that lets one easily inject UI dependencies postfactum. Thus, conditional compilation. (I am not happy about this at all, but again, other things took priority, e.g. the file format being backwards compatible and intricately tied to internal data representation, making either of those nearly impossible to change between versions.)

I hope you can see why other things, like making code actually modular (and not just easier to navigate) were higher priority.

Yep, I'm starting to see that now. I'd like to apologize for simply jumping to conclusions instead of trying to understand how and why the codebase is the way that it is today. Nothing ever happens without a reason, and I should figure those reasons out before I attack them. Also, thank you for being so responsive and answering my questions in detail.

In that vein, how do you feel about javadoc style comments? If you don't like that style, how should I be putting comments in and where (headers, cpp files)?

In that vein, how do you feel about javadoc style comments? If you don't like that style, how should I be putting comments in and where (headers, cpp files)?

Doxygen style comments are 100% fine. I would be very happy to see more documentation, though it should of course be actual documentation and not IDE-generated (or even handwritten) comments of the style /** Performs A. */ void a(), which I occasionally see in projects that try to achieve 100% documentation coverage no matter what quality...

As per discussion above, I'm closing this PR. @marthinwurer, I will be glad to see your other contributions.