The main principle of good code

For twenty years of diverse programming, I formulated, I am convinced, the main principle of good code. Relying on it, I and my colleagues managed to put in order the most terrible code, to unite in a team of incompatible programmers and to maintain the system for years without too much nagging.

Read this article: 15 minutes
Understanding the technique: 10 minutes
Tangible results: 30 minutes

So,

What for?

What about bad code?
')

• nothing is clear
• incomprehensible variables
• huge methods
• non-obvious algorithms
• hacks, encapsulation violations
• high complexity and confusion
• changes lead to surprises
• nothing is clear

It was not for nothing that I pointed out the same moment twice. Indeed, almost everything comes down to the fact that the programmer cannot deal with some code.



Bad code is slow to change, and changes bring surprises. This is understandable: the programmer does not see what the code is doing, and he has to understand instead of creating something new. Actually, it often happens that only its author is really capable of fixing a program.

Therefore, one of the most important features of a good code is its clarity. Understanding is a purely human notion. The compiler doesn’t care what it is called, he doesn’t get to the point; only person reads code. Only a person from the name of the method can imagine what exactly the method does. Only a person reads the name of a variable and immediately sees the essence of the data stored in it.

I am convinced that the clarity of the code is more dependent on the names. From the names of classes, methods, variables. The clearer the names are formulated, and the more, the easier the code is read.

For example, there is such a code for sending a notification to a mobile device:

function locator($device) {
    // Can we locate the device?
    if ($device->token != "" && $device->expire <= $now) {
        return false;
    }

    $modelNotifier = new ModelNotifier($device); 
    return $modelNotifier->go();
}

, - . .

• $device?
• $device->expire $now? ?
• $modelNotifier->go()?

. , , $device->expire , . ModelNotifier->go() , . , , token . , .

, : token!="", . , , .

, . — . , ; , . , .

.

?

, — .

, , , . , — — .

:

function locator($device) {
    // Can we locate the device?
    if ($device->token != "" && $device->expire <= $now) {
        return false;
    }

    $modelNotifier = new ModelNotifier($device); 
    return $modelNotifier->go();
}

, , : “ ?”

:

function ModelDevice::isLocatable() {
    return ($this->token != "" && $this->expire <= $now);
}

…

function locateAndNotifyDevice(ModelDevice $device) {
    if (!$device->isLocatable()) {
        return false;
    }

    $modelNotifier = new ModelNotifier($device);
    return $modelNotifier->go();
}

, , - , .

. isLocatable(). : , :

1. ?
2. ?

: , .

:

function isDeviceExpired() {
    return ($this->expireAtUnixtime >= $now);
}

function isOurTokenValid() {
    return ($this->token != "");
}

function isLocatable() {
    return ($this->isOurTokenValid() && !$this->isDeviceExpired());
}

. : isLocatable() , . isDeviceExpired(), , .

— .

, , , token . , : .

, : . Instant win!

?

, . ! , .

-, . , , . , : . , , . . , ? :)

-, - (“ ?”) (“ ?”) . isLocatable(), : , . — , . “” locateDevice(), ; , , . — , .

-, , CPU, . , — 1/3 ^*. , , . , . , , . , .

^*300ms — .

:

function getConfig() {
    if ($this->hasOption('configs')) {
        $configs = $this->getOption('configs');
    } else if ($this->hasOption('config')) {
        $configs = [ $this->getOption('config') ];
    }

    if (isset($configs)) {
        $root = $this->getOption('root', null);
        if (!$root) {
            if (isset($_SERVER['PWD'])) {
                $root = $_SERVER['PWD'];
            } else if (isset($_SERVER['DOCUMENT_ROOT'])) {
                $root = $_SERVER['DOCUMENT_ROOT'];
            }
        }

        $configs->root = $root;
    }

    return $configs;
}

: ( ), . :

function grabConfiguration() {
    if ($this->hasOption('configs')) {
        return $this->getOption('configs');
    } else if ($this->hasOption('config')) {
        return [ $this->getOption('config') ];
    }
}

function figureOutRootFolder() {
    $root = $this->getOption('root', null);
    if (!$root) {
        $root = $_SERVER['PWD'];
    }
    if (!$root) {
        $root = $_SERVER['DOCUMENT_ROOT'];
    }
    return $root;
}

function getConfigs() {
    $configs = $this->grabConfiguration();  
    if (!is_null($configs)) {
        $configs->rootFolder = $this->figureOutRootFolder();        
    }
    return $configs;
}

iOS:

if ([imageEntry objectForKey:@"fullscreenUrl"]==nil) {
    NSString *kind = [imageEntry objectForKey:@"kind"];
    if ([kind isEqualToString:@"pdf"]) {
        UIImage *i = [UIImage imageNamed:@"galleryPdfIcon"];
        [self.uiImagesCache setObject:i forKey:@(photoIndex)];
        *photoSize = NIPhotoScrollViewPhotoSizeOriginal;
        return i;
    } else if ([kind isEqualToString:@"video"]) {
        UIImage *i = [UIImage imageNamed:@"galleryVideoIcon"];
        [self.uiImagesCache setObject:i forKey:@(photoIndex)];
        *photoSize = NIPhotoScrollViewPhotoSizeOriginal;
        return i;
    } else {
        UIImage *i = [UIImage imageNamed:@"galleryBrokenImage"];
        [self.uiImagesCache setObject:i forKey:@(photoIndex)];
        *photoSize = NIPhotoScrollViewPhotoSizeOriginal;
        return i;
    }
}

, . . :

- (UIImage *) genericImageForKind:(NSString *) imageKind {
    UIImage *candidate = nil;

    if ([kind isEqualToString:@"pdf"]) {
        candidate = [UIImage imageNamed:@"galleryPdfIcon"];
    } else if ([kind isEqualToString:@"video"]) {
        candidate = [UIImage imageNamed:@"galleryVideoIcon"];
    } else {
        candidate = [UIImage imageNamed:@"galleryBrokenImage"];
    }

    return candidate;
}

…

if ([imageEntry objectForKey:@"fullscreenUrl"]==nil) {
    NSString *kind = [imageEntry objectForKey:@"kind"];
    UIImage *genericImage = [self genericImageForKind:kind];
    if (genericImage) {
        [self.uiImagesCache setObject:genericImage forKey:@(photoIndex)];
        *photoSize = NIPhotoScrollViewPhotoSizeOriginal;
    } 

    return nil;
}

SQL . , , , , .

function(err, dResults) {
    if (err) {
        console.log(err);
        callback(err);
        return;
    }

    var i=0, lastDownloaded = null;
    var unixtimes = Object.keys(dResults);
    for(i=0;i<unixtimes.length;i++) {
        if (dResults[unixtimes[i]].isDownloaded) { 
            lastDownloaded = unixtimes[i];
        }
    }

    for(i=0;i<unixtimes.length;i++) {
        if (unixtimes[i]>lastDownloaded) {
            delete dResults[unixtimes[i]];
        }
    }

    self.getOpenPrice(symbol, function(err, openPrice) {
        callback(err, dResults, openPrice);
    });
}

:

function getLastDownloadedUnixtime(entries) {
    var lastDownloadedUnixtime = null;

    var unixtimes = Object.keys(entries);
    unixtimes.forEach(function(unixtime) {
        if (entries[unixtime].isDownloaded) { 
            lastDownloadedUnixtime = unixtime;
        }
    });

    return lastDownloadedUnixtime;
}

function cleanupEntriesNewerThanLastDownloadedUnixtime(entries, lastDownloadedUnixtime) {
    var unixtimes = Object.keys(entries);
    for(var i=0;i<unixtimes.length;i++) {
        if (unixtimes[i]>lastDownloadedUnixtime) {
            delete entries[unixtimes[i]];
        }
    }
}

…

function(err, dResults) {
    if (err) {
        console.log(err);
        callback(err);
        return;
    }

    var lastDownloadedUnixtime = getLastDownloadedUnixtime(dResults);
    cleanupEntriesNewerThanLastDownloadedUnixtime(dResults, lastDownloadedUnixtime);

    self.getOpenPrice(symbol, function(err, openPrice) {
        callback(err, dResults, openPrice);
    });
}

XML Amazon S3. , , XML :

strcat(xml, "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
strcat(xml, "<Delete>\n");
strcat(xml, "<Quiet>true</Quiet>\n");

for (uint32_t i=0;i<batchCount;i++) {
    char *escapedPath=Deleter::xmlEscape(batch[i]);
    strcat(xml, "\t<Object><Key>");
    strcat(xml, escapedPath);
    free(escapedPath);
    strcat(xml, "</Key></Object>\n");
    LOG(LOG_DBG, "[Delete] %s", batch[i]);
}

strcat(xml, "</Delete>\n");

, :

void addPathToDeleteXml(char *xml, char *path) {
    char *escapedPath=Deleter::xmlEscape(path);
    strcat(xml, "\t<Object><Key>");
    strcat(xml, escapedPath);
    strcat(xml, "</Key></Object>\n");
    free(escapedPath);
}

void addBatchOfPathsToXml(char *xml, char **batch, uint32_t batchCount) {
    for (uint32_t i=0;i<batchCount;i++) {
        addPathToDeleteXml(xml, batch[i]);
        LOG(LOG_DBG, "[Delete] %s", batch[i]);
    }
}

…

strcat(xml, "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
strcat(xml, "<Delete>\n");
strcat(xml, "<Quiet>true</Quiet>\n");

addBatchOfPathsToXml(xml, batch, batchCount);

strcat(xml, "</Delete>\n");

, — . : , - . , - , - , , .

, . , . , , , .

. , . , . , .

— . , : , , .

. — , - .

. , , , autocomplete.

Update: «if (true) return true» , .. , .