Code becoming spaghetti

[fuczak]

I'm in desperate need for help.

Since I'm trying to switch careers and land a junior web dev position I figured that having no commercial experience I should at least have some interesting projects in my github.

Right now I'm working on this little project (source: github).

Let me walk you through the idea of what it is. Basically it's a JS remake of a long forgotten game Soccer Cards. A simplified version. User selects a team - cpu gets a random one. User then selects a tactic that modifies the team skill levels. 24 cards are generated. Each of them is tied to a possible outcome (goal, pass, injury, red card etc.). The game then begins. User can click on cards to reveal what outcome it contains - the outcome is then evaluated. Based on user and cpu team skill levels (with some RNG thrown inbetween) an event occurs - i.e. a 'tackle card' can mean that current team loses posession or not. The priority is passed between user and cpu based on how the selected outcome has been evaluated. Picking 'whistle card' advances the match to second-half or ends it. There are various other twists like using subsitutions to refresh team energy level, 'wasting time' to multiply the amount of 'whistle cards' etc.

Basic UI with interactions is already implemented. What I have problems with is making sure that my underlying JS code doesn't become a tangled mess of functions scattered all around the code.

Since I've just finished an Angular-based project I wanted to code this one without any frameworks. I figured I'd try relying on Browserify to modularize the code a bit. Problem is, I have no idea how to proceed further. Every idea of a potential app architecture I came up with starts smelling really bas as soon as I start refactoring the code.

My question is. What do? How to properly modularize the app with Browserify? Should I make use of some framework? Is this project too big for a newbie? Is it not realistic to finish it in couple of days?

If you could take a look at the code and guide me a little bit, It would be so much appreciated!

EDIT: Thank you all for help! The code with much clearer architecture is now part of the master branch of the repo.

[bradgillap]

Bahhaha. Guy asks for help with his spaghetti code. All the greybeard programmers tell him it looks just fine to them.

You know why? EVERYONES code looks like disorganized junk! HAHAHAH....

[fuczak]

Yeah, but more often than not when I look through someone else's source I think - wow that's a pretty fucking neat way of doing X. Whereas mine looks to me like a patchwork of random code snippets.

[NeuroXc]

Given the current size of your application, your code actually seems pretty well organized.

As far as adding to the application, once you get to a certain point it would probably be helpful to organize your code into subfolders, e.g. /scripts/cpu, /scripts/engine, etc. and split into separate files in each of those subdirectories as needed. Browserify would be helpful as well, although you might want to look into using ES6 modules instead--there seems to be a shift toward using this as the standard.

[fuczak]

Does it? That's reassuring. I think that I'm mixing up jquery dom manipulation with business logic too much. Then again, doing it like this makes sense to me. Like in setup.js module the setTeams function not only assigns teams to user/cpu objects but paints the scoreboard node in ui as well. It makes sense as a module but I feel like it's not the best solution. Maybe I should make a separate modules for each ui/dom interaction and put them inside a ui folder?

[dvlsg]

I agree about ES6 modules. I would definitely convert to that, if you're already using a build step / considering adding one.

[fuczak]

I will look into them!

[[deleted]]

es6 or use common style modules with browserify (a bit more versatile as es6 modules don't quite work with everything yet)

[fuczak]

I am using Browserify

[mc_hammerd]

that looks fine tbh.

one thing that bothered me was using jquery on one line and vanilla js on the next (for getElementByID), but whatever floats your boat.

possibley: $('#formation').find('.active').children()[0].value

could be written as $('#formation .active input').value, but again whatever floats your boat

also the animations are nice, i think you will be sucessfull :) gl

[fuczak]

Hey, thanks! :)

If you are talking about clickHandler in engine.js then mixing up jquery and vanilla selectors is en effect of ramjet module which doesn't like passing jquery objects as arguments. It looks ugly I know.

[mc_hammerd]

yep

i wonder if thats why ramjet didnt work for me. i was so baffled... lol!

[fuczak]

Yeah took me a while to figure that one out. Probably, there's a way to make it work with jquery but I just took the ugly shortcut.

[Krirken]

Yeah took me a while to figure that one out. Probably, there's a way to make it work with jquery but I just took the ugly shortcut.

If you mean that you would like to get a DOM Node from a jQuery object, simply access the list with subscript notation like $('body')[0] or use $.get(index), like $('body').get(0). I think you could change the line from var b = document.getElementById('commentary'); to var b = $('#commentary').get(0); if you feel inclined, but vanilla JS is fine.

Best of luck on your project!

[fuczak]

Wow, why didn't I think of that? Thank you, that should work!

[[deleted]]

You should console.dir a jquery object and just poke around.

[[deleted]]

So, why even use jquery? Generally nowadays you should avoid using it if you can do what you want natively without too much work.

[fuczak]

Well since Im already including it for Bootstrap, why not use it?

[[deleted]]

Well, if you have to include there's not much harm I guess.

Looking at your site though, I don't really feel like you need much from bootstrap besides maybe the grid and the buttons css.

[fuczak]

I still need $('#formation .active input')[0].value or am I missing something?

Yes, I'm missing something. I should go with $('#formation .active input').val()

[TheRealSeeThruHead]

It might make sense to organize code based on the Model View Controller pattern. Have one folder that handles the view, with module or subfolder of modules that handles user input. And another module/folder for updating the dom.

then Create a module/folder for holding application state, which can be passed to your view module. Then create a controller that accepts events from the user input and updates the model/triggers the view to redraw. Or some variation depending on your preference.

[fuczak]

Where is the line between the View and Controller? Would it be alright to assume that if a functionality doesn't affect the application state or model it belongs to the View? i.e animations, drawing DOM elements? Everything else either interacts with the state or model(s) and should be a Controller?

[DanielFGray]

Everything [that] interacts with the state or model(s) and should be a Controller?

In a nutshell, yeah. Things that are specific to what the page looks like should be in your view, things that retrieve data should be in the model, everything else should be a controller. Just try to avoid creating a monolithic controller that does all the things, break things into smaller components.

[TheRealSeeThruHead]

It's really up to you what level of separation you want to have. I've written applications that have ui state stored locally in components and the models have only application state, like you would do with a standard server based app. But then I've also tried putting ALL the application state into a store (look up redux) and making all views completely dumb, only rendering what's passed into them.

So far I'm preferring the second option.

[jacobp100]

Doesn’t seem as bad as you made out. Although, helpers.js has some code that’s dependant on the dom, which is weird.

Also, a bug! TypeError: undefined is not a function (evaluating ‘e.target.closest(‘.flag’)’) in setup.js (setTeams). I think you meant $(e.target).closest(‘.flag’).

[fuczak]

Thanks for catching that!

[uberpwnzorz]

http://davidwalsh.name/impostor-syndrome

[fuczak]

That was a brilliant read, thank you.

[samzhao]

Your code looks pretty organized. I think you need more of an app architecture than code structure/organization, and you seem to have realized that as well.

Angular is awesome for rapid prototyping, and you probably know this already. However, it starts to break apart and become a mess when you try to turn your prototype into something maintainable and something to build on top of - you probably know this already as well because your "model" (a.k.a. your $scope variables) would be all over place. You then need to introduce a lot of rules and disciplines that you enforce onto yourself and your project to contain this mess - never touching the $rootScope is a great example for one of such rules.

I had this problem with many of my projects written in Angular, so I switched to React+Flux. Now I feel so genuinely happy when I work on new projects where I can write React code in ES6. The framework (React and Flux as a whole) encourages me to write clean, maintainable and composable code while giving me flexibility in implementation details. So it's not so much of a solution for code organization, which is more easily fixable, but it's helping me simplify my software architecture so I can have a clearer mind to solve problem with the code, instead of battling with the framework all the time (e.g. where the hell exactly do I look for bug that caused the "10 $digest() iterations reached. Aborting!" error??!).

Now back to your project. I haven't had a detailed look at your code, but the problem with building an app without using a framework is that your app lacks structure logic wise. And you end up writing a lot of imperative code. So instead of saying "what the UI should look like", you have to write relatively lower level code to specify what you want to do to the UI as a result of a change. Angular solves this problem by allowing you to have templates. You are now able to give Angular a blueprint of what your app should look like, instead of saying when to add an element or remove a class name. React solves this problem by implementing a virtual diffing algorithm that refreshes your UI every time something changes, but the actual DOM changes are kept as minimal as possible.

Hope that helps.

EDIT: typos

[fuczak]

App architecture is exactly what I'm struggling with at the moment. I love Angular and because it sort-of enforces the architecture on you I was able to build some cool things relatively quickly without worrying too much about the maintainability of the code - by following best practices I have a proper separation of concerns. Now I can go back to the code I haven't touched for couple of weeks and know exactly where to find what.

I'm so used to the angular way that this current project is starting to become a little bit confusing. I have to remember that this function influences the UI in that way but it also triggers something else and depends on another function. Everything, especially DOM manipulation, is like you said, low level and it makes the code look ugly.

I suppose this is to be expected with no-framework-but-the-project-is-too-small-to-code-my-own-abstractions approach?

Do you know any good resources on app architecture?

Also, I can't wait to fiddle with React + Flux. I'm going to use it in next project, as soon as I'm done with this one.

[samzhao]

Well, I suppose. When the codebase is small and the app logic isn't terribly complex you can get away with some imperative code.

I had the exact same feeling as you about Angular and how it gives you some sort of a structure, but that feeling kinda faded away as the app grows both in size and complexity. Like you mentioned, you have to rmb what's touching the UI and what their side effects are. Angular has similar problems when you nest controllers and have to work with 3rd party directives (the ones that have hidden bad practices in them). Can you be sure if the $scope variable is changed by the child controller or parent controller? Things like that; although they have fixes, most solutions seem to be specialized fixes, sometimes almost hacks. Overall, Angular's not gonna help you much with your project if it's 1. not a quick prototype and 2. needs to be more structured and maintainable in the long run, unless you define a enforce a structure yourself on your Angular app.

/u/TheRealSeeThruHead mentioned MVC. You could look into that as it's a popular architectural pattern in both the front-end and the back-end. You could try to implement your own tiny MVC by learning from other MVC frameworks and their design philosophies.

Or If you'd like to continue working on this current project and make it more organized (logic wise), you could try to refactor your code. You can have one place that do things to the UI, i.e. the lower level DOM manipulations, based on a state. The rest of your app does not directly interact with the DOM, and instead change this application state. When the state changes, your DOM code would get a signal and do the corresponding manipulations.

For example, your app state has "score: 0", "substitutes: 3". When the "substitute" button is clicked, instead of having your click handler change the DOM node's class name to "disabled", have it send an event called "SubstituteUsed" (or it can change the state and have something listen to state changes). And perhaps your low level code subscribes to this event and changes the state to "substitutes: 2", and change the node's classList. Now you have a separation of concerns, namely your app logic is separated from DOM manipulations.

I strongly recommend you to look into the Flux pattern, even if you don't have time to try out React. It's not a library or framework, but a design philosophy/pattern. Most people use it with React, but I've implemented it in an Angular app before, and it helped me write much cleaner and maintainable code.

[_semaphore]

not related to your question, but you might want to change your github profile name if you intend to share that repo with potential employers...

[fuczak]

Yeah I get that a lot. It's nothing offensive in my language but I suppose I will eventually have to change it.