Short code vs "grammatically correct code", opinions?

Page: 1 2 3
May 17 2008, 2:19 pm
In response to Obs
Old school with uncensored comments. ;)
May 17 2008, 2:38 pm
In response to Fint
Fint wrote:
Besides, I think there is also a big difference between code that one person uses or code that a team is supposed to work on.

Not really, because if a few months or years down the road someone has to come back and try to understand the code, it doesn't matter whether its your or someone else. If you used easy to read code, it'll be easy to understand, and if you didn't, it'll be a headache.

I hate headaches.

Since everyone else is posting chunks of their code, I might as well throw in my fair share. I've decided that I'm a big fan of white space and line breaks. I dislike having multiple lines merged into one, and I like all of my variables to be reasonably straightforward, although I use a few generic letter variables for common things such as "M" usually referring to the targeted mob.

I'm also happy to say that I find my code very easy to understand when I come back to it several months down the road.

/*
Multi-fleet encounter outline:
- Organize fleets by initiative:
- Calculate the total ships for each fleet
- For each round while more than 1 fleet has ships:
    - For each ship class (ordered by initiative):
       - Have each fleet choose another fleet to attack
       - Attack those ships
    - Recalculate the ship counts for each fleet
- Determine the winner
*/
planet/proc/Conflict(list/fleetlist)
    var/min_flux = 0.5 // minimum variation in damage
    var/max_flux = 1.5 // maximum variation

    // Make sure that each argument is actually a fleet and create a new list of fleets.
    var/list/fleets = list()
    for(var/fleet in fleetlist)
        if(istype(fleet, /fleet))
            fleets += fleet

    // Organize the fleets list by their initiative,
    // the fleet with the most initiative being first.
    QuickSort(fleets, /proc/FleetSortByInitiative)

    // Calculate the total ship counts for each fleet as well as
    // the hit points for each of that fleet's ship classes.
    for(var/fleet/fleet in fleets)
        fleet.CalcFleetHealth()

    // While there is more than one fleet in the conflict:
    while(fleets.len > 1)

        // Loop through each ship class in a sorted list of ships, where
        // ships with the most initiative go first.
        for(var/ship_class/class in ship_initiative_sort)

            // And have each fleet choose another fleet to attack with
            // that particular ship class.
            for(var/fleet/fleet in fleets)

                // If this fleet has none of that ship, skip on to the next fleet.
                if(!fleet.ships[class])
                    continue
                if(fleet.ship_health[class] <= 0)
                    continue

                // Build a list of fleets which are decidedly enemies
                // of this fleet, then choose one of those fleets to attack.
                var/list/enemy_fleets = list()
                for(var/fleet/enemy in fleets)
                    if(enemy.owner != fleet.owner)
                        enemy_fleets += enemy
                var/fleet/enemy = pick(enemy_fleets)

                // *** Damage Calculations ***
                // Calculate how many ships are remaining.
                var/ships = round(fleet.ship_health[class] / class.hitpoints, 1)
                var/modifier = 1

                // Fleets get damage bonuses for multiple ship types.
                modifier += (fleet.ships.len - 1) / 10

                // If the fleet's owner also owns the planet, then the fleet's
                // damage is multiplied by the planet's defense values.
                if(fleet.owner == src.owner)
                    modifier *= src.GetDefenseBonus()


                // Loop through the current ship class's target list to see which enemy
                // ships these ships want to target first.
                for(var/ship_class/target in class.targetlist)

                    // If the enemy fleet has none of this target, skip on to the next target.
                    if(!enemy.ships[target])
                        continue
                    if(enemy.ship_health[target] <= 0)
                        continue

                    // Calculate and randomize damage!
                    var/damage = class.CalcDamage(target) * ships * modifier
                    enemy.ship_health[target] -= rand(damage * min_flux, damage * max_flux)

                    // Unfortunately we only get to attack one ship class.
                    break

        // Now that the first wave of the conflict has ended, see how many
        // ships of each class are remaining in each fleet.
        for(var/fleet/fleet in fleets)
            for(var/ship_class/class in fleet.ships)
                fleet.ships[class] = max(0, round(fleet.ship_health[class] / class.hitpoints, 1))

            // If the fleet has dwindled down to nothing, then remove it
            // from the fleets list and destroy the fleet.
            if(fleet.TotalShips() <= 0)
                fleets -= fleet
                //fleet.Destroy()

        if(fleets.len > 1)
            // Check to see if all of the remaining fleets are on the same team
            var/list/teams = list()
            for(var/fleet/fleet in fleets)
                teams["[fleet.owner]"]++

            // If they are all on the same team, then merge all the teams
            // until there is only one team left so the battle will end.
            if(teams.len == 1) // only one team left
                while(fleets.len > 1)
                    var/fleet/allies = fleets[2]
                    fleets -= allies
                    allies.Merge(fleets[1])


    // Now return the only remaining fleet.
    return fleets[1]
May 17 2008, 2:47 pm
In response to Popisfizzy
ah, forgot that

many of my comments are actually laced with some profanity. A clear sign of past frustrations.
May 17 2008, 4:11 pm
In response to Foomer
I for one, almost never use comments unless it's a hacky-piece of code I know I'm going to have trouble understanding later on.

Whitespace is all well and good, but generally I'll only use it to separate procs in the one group as opposed to inside the proc itself.

Generally I don't have any trouble reading my own code.

Obligated snippet.
General_Interface
    var/client/caller = null
    var/current_window = null
    proc
        open_window(window)
            if(src.current_window) return alert(caller, errors[4], "Error \[4]")
            winset(caller, window, "is-disabled=false;is-visible=true")
            src.current_window = window
        
        close_window(window)
            winset(caller, window, "is-disabled=true;is-visible=false")
            src.current_window = null
        
        reset_window(window)
            switch(window)
                if("new_window")
                    for(var/A in list("firstname", "surname", "biography")) caller << output(null, "[window].new_[A]")
                    for(var/A in list("race_human", "class_warrior", "gender_male", "alignment_neutral")) winset(caller, "[window].[A]", "is-checked=true")
                if("load_window")
                    for(var/A in list("name", "race", "gender", "class", "avatar", "load", "delete"))
                        for(var/i = 1 to 3)
                            switch(A)
                                if("load", "delete") winset(caller, "[window].character[i]_[A]", "command=[null]")
                                if("avatar") winset(caller, "[window].character[i]_[A]", "image=[null]")
                                if("name", "race", "gender", "class") winset(caller, "[window].character[i]_[A]", "text=[null]")
                if("battle_child")
                    for(var/A in list("player", "attacker"))
                        winset(caller, "[window].[A]_health", "size=[winget(caller, "[window].[A]_maxhealth", "size")]")
                        winset(caller, "[window].[A]_mana", "size=[winget(caller, "[window].[A]_maxmana", "size")]")
                        winset(caller, "[window].[A]_avatar", "image=[null]")
                        winset(caller, "[window].[A]_healtht", "text=[null]")
                        winset(caller, "[window].[A]_manat", "text=[null]")
        
        is_checked(window, list) for(var/A in list) if(winget(caller, "[window].[A]", "is-checked") == "true") return copytext(A, findtext(A, "_")+1)
May 18 2008, 6:17 am
In response to Tiberath
Sometimes I compact lines that are closely related. Mostly combining variable assignments that describe something singular, like coordinates. Most of the time I don't, though. Generally, I prefer whitespace in and around my procs. Usually, I space procs two newlines apart, and larger code sections with 3 newlines. One newline separates sections of procedures that are not closely related, to give my eyes some downtime between reading lines of code. I have been accused of using whitespace too much, but these people are not worth listening to, in my opinion.

Comments, on the other hand, I am not so good about. For the most part, I take the position that if wrote it, I should have little trouble reading it and understanding what's going on. It's a self-delusion, I know. Many times I've gone back and was completely lost in my own illogical statements. For this, and other reasons stated here in this thread, I try and comment when it seems appropriate. I'll admit, I'm still learning when that appropriate time is. :/

Here's a couple procs from Chatters' source, to show some of my better programming habits.

proc
    Implode(words[], seperator)
         // if words isn't a list, or no seperator, return
        if(!istype(words,/list) || !seperator) return

        var/text = "" // To store our new string

        for(var/w=1, w<=words.len, w++)  // for each position in the list
            if(w!=words.len)            // if it's not the last position
                text += "[words[w]][seperator]" // add it to the text with a seporator

            else                        // Otherwise
                text += "[words[w]]"    // don't add the seperator

        return text // return the converted text


    Explode(text, seperator)
        if(!text || !seperator) return // if no text or seperator, return

        var
            List[] = new() // List holds the words to pass back
            pointer = 1 // The current character position in the text
            nextpos = findtext(text, seperator, pointer) // the next position

        do // do the conversion (see while)
            if(!nextpos || nextpos==length(text)) // if no seperator found (or is the end)
                List += copytext(text, pointer) // add the remainder to the list
                nextpos = null // and set nextpos to null to break the loop

            else // Otherwise
                List += copytext(text, pointer, nextpos) // add the item to the List
                pointer = nextpos+1 // update our pointer
                nextpos = findtext(text, seperator, pointer) // and look for the nextpos
                if(!nextpos) nextpos = length(text)

        while(nextpos) // while there is a next position

        return List // After all that, return the List


May 18 2008, 5:56 pm
In response to Xooxer
Xooxer wrote:
Generally, I prefer whitespace in and around my procs. Usually, I space procs two newlines apart, and larger code sections with 3 newlines.

I don't like adding whitespace when a proc is very straight forward. As you can see with my above is_checked() proc.

is_checked(window, list) for(var/A in list) if(winget(caller, "[window].[A]", "is-checked") == "true") return copytext(A, findtext(A, "_")+1)
 
is_checked(window, list) 
    for(var/A in list) 
        if(winget(caller, "[window].[A]", "is-checked") == "true") 
            return copytext(A, findtext(A, "_")+1)


I'd prefer to have it in one line as opposed to three or four.

However, when a proc gets difficult, I'll write it using mass amounts of whitespace per section (for example anything that uses a switch). But afterwards I'll remove the non-necessary whitespace within the proc itself.
May 18 2008, 6:47 pm
In response to Tiberath
I'd infinitely prefer the multi-line one over the one-liner. Very straightforward code isn't very straightforward after its compressed. If its straightforward, then LEAVE IT STRAIGHTFORWARD!

If you find it straightforward because its a proc that you're familiar with, then it still doesn't help if you make it hard for someone else to read, since you never know when someone else might read your code. You also never know when your programming style might change and a once familiar proc becomes something you're not used to anymore.
May 18 2008, 7:44 pm
In response to Tiberath
Tiberath wrote:
I'd prefer to have it in one line as opposed to three or four.

However, when a proc gets difficult, I'll write it using mass amounts of whitespace per section (for example anything that uses a switch). But afterwards I'll remove the non-necessary whitespace within the proc itself.

In the given example, I'd definitely use the four-line version myself for the express reason that the other forces horizontal scrolling. I somewhat strictly enforce an 80-horizontal-character limit in my source code (tab stops at 2 spaces) because horizontal scrolling is one of the most time-consuming operations -- you can't use a scrollwheel to do it unless the application is specifically designed to enable it, and having to move directly to the scrollbar in order to manually drag the slider every time you want to see what's off the screen is a huge headache. =)
May 19 2008, 3:27 pm
In response to Foomer
Foomer wrote:
I'd infinitely prefer the multi-line one over the one-liner. Very straightforward code isn't very straightforward after its compressed. If its straightforward, then LEAVE IT STRAIGHTFORWARD!

If you find it straightforward because its a proc that you're familiar with, then it still doesn't help if you make it hard for someone else to read, since you never know when someone else might read your code. You also never know when your programming style might change and a once familiar proc becomes something you're not used to anymore.
tell that to lummox JR. his Runt Source is like impossible to understand. it's like... all one line.
May 19 2008, 3:34 pm
In response to Eternal Desire
That's on purpose. The 4k contest has to have source code under four kilobytes, and the best way to do that is to condense it down like that.
May 19 2008, 3:41 pm
In response to Popisfizzy
Popisfizzy wrote:
That's on purpose. The 4k contest has to have source code under four kilobytes, and the best way to do that is to condense it down like that.

well it's pretty much impossible to understand.

i'd like to know how ANYONE understands it.
May 19 2008, 4:14 pm
In response to Eternal Desire
It's not meant to be understood.
May 19 2008, 4:26 pm
In response to Garthor
Garthor wrote:
It's not meant to be understood.

why release it, then?
May 19 2008, 4:26 pm
In response to Eternal Desire
He had to for the 4k contest.
May 19 2008, 4:28 pm
In response to Popisfizzy
Popisfizzy wrote:
He had to for the 4k contest.

well i feel bad for anyone that had to read the migraine-inducing 'thing' called the runt source.
May 19 2008, 4:30 pm
In response to Eternal Desire
Thing? It's still better-programmed than you or I could do. Just because it's compressed doesn't make it poorly programmed. It just makes it difficult to read.

[Edit]
It probably employs practices Lummox normally wouldn't do, though, as they shorten the length of his code, as is the nature of the 4k contest.
May 19 2008, 4:33 pm
In response to Popisfizzy
Popisfizzy wrote:
Thing? It's still better-programmed than you or I could do. Just because it's compressed doesn't make it poorly programmed. It just makes it difficult to read.

i'd sooner yank my tounge out and shove it before i code anything that stupid.

mob/verb/say(t as text) world<<"[usr]: [t]

that doesnt save ANY space at all.

and it's harder on the eyes.
May 19 2008, 4:35 pm
In response to Eternal Desire
Nobody had to read it. The challenge of the contest was to write a game where the source code is less than 4 kilobytes in size. In order to verify this, the source code is provided, so the judges can say, "Okay, that's less than 4 kilobytes, and it compiles, so it's a valid source code." They aren't reading it.
May 19 2008, 7:05 pm
In response to Eternal Desire
Eternal Desire wrote:
tell that to lummox JR. his Runt Source is like impossible to understand. it's like... all one line.

The source code for Runt that's publicly downloadable isn't the source code that Lummox wrote. Lummox first created the game, and then he took a series of steps to condense the code so that it took up as little space as possible.

Basically its the equivalent of taking a text file and running it through a zip program, then opening it up in a text editor. Yes, it looks like jibberish, because it is. But it wasn't written as jibberish, it looks like that because its compressed.
May 19 2008, 7:36 pm
In response to Foomer
Foomer wrote:
Eternal Desire wrote:
tell that to lummox JR. his Runt Source is like impossible to understand. it's like... all one line.

The source code for Runt that's publicly downloadable isn't the source code that Lummox wrote. Lummox first created the game, and then he took a series of steps to condense the code so that it took up as little space as possible.

Basically its the equivalent of taking a text file and running it through a zip program, then opening it up in a text editor. Yes, it looks like jibberish, because it is. But it wasn't written as jibberish, it looks like that because its compressed.

I wonder what the original Runt source looks like, then.
Page: 1 2 3