Simple timer class for game model I have a simple strategy game, and I implemented a simple counter object to count down a number whenever the game ticks.  I have heard about something called NSTimer but I haven't messed with it at all yet.  I have also heard that you should use Core Animations to move objects around when rendering rather than a tick based system, because tick based movement would be jerky.  However I wanted to make sure to handle animation and rendering separately from the underlying game model, so that part doesn't matter as much.  This is just to be used in the game model, and the actual animations of everything will be handled by the scene.

I realize I am probably reinventing the wheel here, but I wanted to build something dead simple that I could use in a few places right away, and I wanted the experience.  I am hoping most of all that the class is easy to understand, so let me know what you think.

The scene updates the game object according to the following method:

- (void)update:(NSTimeInterval)currentTime {
    CFTimeInterval timeSinceLast = currentTime - self.lastUpdateTimeInterval;

    //update game and hud counters every quarter second
    if (!_game.isPaused) {
        self.countdownInterval += timeSinceLast;
        if (self.countdownInterval > 0.25) {
            [self updateGame];
            [self buildResourceCounters];
            self.countdownInterval = 0;
        }
    }

    self.lastUpdateTimeInterval = currentTime;
    if (timeSinceLast > 1) { // more than a second since last update
        self.lastUpdateTimeInterval = currentTime;
    }
}

And here is the Countdown class which the game will update each tick:

typedef NS_ENUM(NSInteger, CountdownState){
    CountdownNotStarted = 0,
    CountdownCounting,
    CountdownPaused,
    CountdownFinished
};

DTCountdown.h:

#import <Foundation/Foundation.h>
#import "DTCountdownStates.h"

@interface DTCountdown : NSObject

-(id) initWithCount:(int)startCount;

-(void) update;
-(void) startCountdown;
-(void) pauseCountdown;
-(void) restartCountdown;
-(void) cancelCountdown;

@property CountdownState state;

@end

DTCountdown.m:

#import "DTCountdown.h"

@implementation DTCountdown {
    int _currentCount;
    int _startCount;
}

-(id) initWithCount:(int)startCount {
    self = [super init];
    if (self) {
        _startCount = startCount;
        _currentCount = _startCount;
        _state = CountdownNotStarted;
    }
    return self;
}

#pragma mark - Update Loop
-(void) update {
    switch (self.state) {
        case CountdownNotStarted:
            break;
        case CountdownCounting:
            [self doCountdown];
            break;
        case CountdownPaused:
            break;
        case CountdownFinished:
            break;
        default:
            break;
    }
}
-(void) doCountdown {
    if (_currentCount > 0) {
        _currentCount--;
    } else {
        self.state = CountdownFinished;
    }
}

-(void) startCountdown {
    self.state = CountdownCounting;
}
-(void) pauseCountdown {
    self.state = CountdownPaused;
}
-(void) restartCountdown {
    _currentCount = _startCount;
    self.state = CountdownCounting;
}
-(void) cancelCountdown {
    _currentCount = _startCount;
    self.state = CountdownNotStarted;
}

@end

There's one style issue I want to address, and then I'll talk bigger picture.

This comment may turn irrelevant in this specific case, because bigger picture, the better option is probably going to be to use NSTimer, and I'll go over some samples of how to use that, but this comment is still useful for other scenarios, so here it goes:

-(void) update {
    switch (self.state) {
        case CountdownNotStarted:
            break;
        case CountdownCounting:
            [self doCountdown];
            break;
        case CountdownPaused:
            break;
        case CountdownFinished:
            break;
        default:
            break;
    }
}

We have 4 possible conditions (not including default:), but only 2 possible results.  What you're doing here is on par with something like this:

NSUInteger foo;
// stuff

switch (foo) {
    case 0: break;
    case 1: [self doCountdown];
    case 2: break;
    case 3: break;
    case 4: break;
    // etc ad naseum
    case NSUIntegerMax: break;
    default: break;
}

Listing EVERY possible value for an NSUInteger in a switch statement in which we only care about 1 (or even a couple) value seems absurd, right?  It's no less absurd to do this for an NS_ENUM.

After all, your NS_ENUM is actually backed by some sort of primitive data type.  And in this case we have only 4 different values, but it doesn't mean there can't be more.  For my current project at work, one of my enums already has 100+ defined values, and will easily have hundreds more by the time the project is done, yet most of the time when I switch on variables of this type, I only care about one specific value, sometimes two or three more.

There are three possible options on how to rewrite this into something better.

A simple if-else.  If the specific scenario means it's extraordinarily unlikely that we'll only ever care whether or not the variable matches this one specific variable, and we don't need an if-else if-else or anything more complex than a simple if-else, then we're fine to just use an if-else:

if (self.state == CountdownCounting) {
    [self doCountdown];
}

And in this specific case, we don't even need the else.

Let conditions with that should execute the same block of code simply fallthrough:

switch(self.state) {
    case CountdownCounting:
        [self doCountdown];
        break;
    case CountdownNotStarted:
    case CountdownPaused:
    case CountdownFinished:
    default:
        break;
}

This one explicitly lists each condition, and this is fine if the enum is well defined, small, and unlikely to contain more than a handful of values.  However, this specific syntax will not work with swift switch statement.  You'd have to add the fallthrough keyword to each of those empty cases to achieve the exact same code, and at that point, we're not any better than the original situation of just break-ing from each.  Since iOS development will likely move quickly to swift, I'd recommend against getting into the habit of this sort of switch structure if you're not already in it.

Let default: handle the default cases.  Any case for which the code you want to be executed matches exactly to the code in default: can simply be omitted.

switch (self.state) {
    case CountdownCounting:
        [self doCountdown];
    default:
        break;
}

Now this is the same logic as the if-else above.  It makes it more clear that we only care about this specific case.  However, compared to the if-else above, this is easier to add other values and scenarios.  And this structure will work perfectly fine exactly as-is barring syntax discrepancies if/when you make the transition to swift.  This is absolutely the style I would recommend, although all three options are better than what you currently have I believe.

Now then, as I mentioned before (and as you mentioned in the question), NSTimer is probably the answer here.

You said you'd heard of it, but not messed with it yet.  NSTimer is no different from any of the other Apple classes.  It may seem a little confusing or overwhelming at first, but the documentation is great, and it really only takes using it for yourself once or twice to get pretty settled in and comfortable with how the class works, so here's a few pointers on NSTimer.

There are 5 methods for creating a timer.  Of these 5, I personally only ever use 1 of them.  One of the methods is an init method, where the other 4 are factory methods, convenience methods that do some extra set up for you.

Of these 4 factory methods, there's two groups, scheduled and unscheduled.  The two methods that start with the phrase timerWithTimeInterval: create unscheduled timers.  The two that start with the phrase scheduledTimerWithTimeInterval: are scheduled timers.  The only difference between creating timers with these two groups is that the former need to be scheduled after they've been created while the latter do not.  The latter are scheduled to start counting time from the instant they're instantiated.

So let's focus on the two scheduled timers as these are the ones that are really the most useful.

+ (NSTimer *)scheduledTimerWithTimeInterval:(NSTimeInterval)seconds
                                 invocation:(NSInvocation *)invocation
                                    repeats:(BOOL)repeats;

+ (NSTimer *)scheduledTimerWithTimerInverval:(NSTimeInterval)seconds
                                      target:(id)target
                                    selector:(SEL)aSelector
                                     repeats:(BOOL)repeats;

So, there are three parts to these timers.

The timer interval between now and when the timer should fire.
The action that should be taken when the timer fires.
Whether or not the action should repeat.

The first method solves part two with a single argument, an NSInvocation class, something I'm not familiar with, but it's basically an Objective-C message rendered static--an action turned into an argument.

The second method solves part two with two arguments.  A target and a selector.  The target is a reference to the object you want to call a method on, and the selector is the method you want the timer to call on that target.

The method should take a single argument of type NSTimer as the timer object sends itself as the argument to the method it calls.  This is particularly useful because NSTimer has a NSDictionary property called userInfo in which you can store any information you want when you create the timer so that the method that gets called can retrieve the information stored.

So, a quick rewrite of your class using NSTimer might look something like this:

In interface:

@property (nonatomic,strong) NSTimer *timer;

In implementation:

- (void)startTimer {
    self.state = CountdownCounting;
    self.timer = [NSTimer scheduledTimerWithInterval:0.25f
                                              target:self
                                            selector:@selector(update:)
                                             repeats:YES];
}

- (void)restartTimer {
    [self cancelTimer];
    [self startTimer];
}

- (void)cancelTimer {
    if (self.timer.isValid) {
        [self.timer invalidate];
    }
    self.state = CountdownNotCounting;
}

- (void)update:(NSTimer *)timer {
    // logic for whatever you want to happen each time the timer fires
}

In the update: method, you don't need any special logic relating to the timer.

When we call invalidate on the timer, it stops the timer.  Because we sent YES for the repeats argument, it will automatically recall update: every 0.25 seconds until we stop the timer.

Although, as you can see, NSTimer is really quite simple to use, and arguably doesn't warrant a wrapper class.

One important thing to watch out for with NSTimer however.  While the timer is valid, the timer object holds a strong reference to the value sent for the target: argument.  This is important to note because in most cases the timer is a property on a given object with a strong reference to it (so the timer doesn't deallocate) and the object will typically send itself as the target argument.  So we're knowingly and intentionally creating a retain cycle for the timer period during which the timer is valid.

This is mostly fine.  If we sent NO as the argument for repeats:, then the retain cycle only lasts until the timer fires, at which point the method is called on the object (which is better than a method being called on a deallocated object, which would crash the app).  Once the method returns, NSTimer releases its strong reference to the object, the retain cycle is broken, the object deallocates, leaving no strong references to the NSTimer object, so it deallocates, and now everything is fine.

NSTimer also breaks its strong reference when it is invalidated however, so all you have to do to break an NSTimer retain cycle is simply call invalidate on it.  This is important to remember when you've sent YES for the repeats: argument.


A temporary directory that exists only during run time of user-specified code For a research analysis, I'm writing a small bash script that serves as a frontend to Singularity. The reason is that I want to save in this script which options are needed for singularity. For example, I want the working directory to appear at a fixed path in the container, regardless of the actual working directory path.

Singularity has the --contain option for this, but this requires me to manually define a temporary directory for the container. I use mktemp for that. After the end of the script, I use trap to
delete that directory. However, I fear that there might be a corner case where trap "rm -rf '$tmpdir' might delete the wrong directory.

The script, called cexec, executes an arbitrary command inside the container. For example ./cexec R starts R inside the container.

Is there a corner case in the following script where the script deletes a directory that it didn't create?

#!/bin/bash
# Execute a command in the container
set -ue

thisdir="$(dirname "$BASH_SOURCE")"
container="rserver_200211_commitd117c677.sif" # get such a file by using `singularity pull ...`

# Create a temporary directory
tmpdir="$(mktemp -d -t cexec-XXXXXXXX)"
# We delete this directory afterwards, so its important that $tmpdir
# really has the path to an empty, temporary dir, and nothing else!
# (for example empty string or home dir)
if [[ ! "$tmpdir" || ! -d "$tmpdir" ]]; then
  echo "Error: Could not create temp dir $tmpdir"
  exit 1
fi
# check if temp dir is empty
tmpcontent="$(ls -A "$tmpdir")"
if [ ! -z "$tmpcontent" ]; then
  echo "Error: Temp dir '$tmpdir' is not empty"
  exit 1
fi
# Delete the temporary directory after the end of the script
trap "rm -rf '$tmpdir'" EXIT

singularity exec \
  -B "$tmpdir:/tmp" \
  --contain \
  -H "$thisdir:/data" \
  "$container" \
  "$@"

Always set up the trap before trying to create the directory. Otherwise there's a race condition where the script may die after creating the directory but before having a chance to clean it up. And use single quotes for the trap command to make sure the variable is only expanded at exit:

trap 'rm -rf "$tmpdir"' EXIT
tmpdir="$(mktemp -d)"

If the trap ends up being triggered before mktemp the result is simply rm -rf "", which does nothing.

Some other suggestions:

#!/usr/bin/env bash is a more portable shebang.
set -o errexit -o nounset is more readable than set -ue.
BASH_SOURCE is an array, and it's only by accident that $array_name refers to the first element of array_name. directory="$(dirname "${BASH_SOURCE[0]}")" would be more explicit.
if [[ ! "$tmpdir" || ! -d "$tmpdir" ]] is redundant. If mktemp fails the script will stop there, and if it's been removed rm -rf "$tmpdir" is safe, as mentioned.
Don't use ls in scripts!
The temporary directory can't already contain any files. That's part of the contract of mktemp - it will create a new directory if it can, or fail otherwise.


Message class, with constructors for data from user and from database I'm fairly new to Kotlin, and am looking to slim down a data class.

It has two constructors, one for creation based on user input, and one for creation based on database data.

open class Message : Unique {

    val timestamp: Long
    val contents: String

    constructor(contents: String) : super() {
        validateMessageContents(contents)
        this.contents = contents
        this.timestamp = System.currentTimeMillis()
    }

    constructor(id: String, contents: String, timestamp: Long) : super(id) {
        validateMessageContents(contents)
        this.contents = contents
        this.timestamp = timestamp
    }

    companion object {
        private const val MAX_MESSAGE_LENGTH = 1024

        private fun validateMessageContents(contents: String) {
            if (contents.isEmpty()) {
                throw IllegalArgumentException("Contents cannot be blank/empty")
            }
            if (contents.length > MAX_MESSAGE_LENGTH) {
                throw IllegalArgumentException("Message too long - please limit to $MAX_MESSAGE_LENGTH characters.")
            }
        }
    }
}

The Unique superclass has a single id: String property, but will generate a random UUID if the default constructor is called.

I'm wondering - how can I slim down this class? I've fiddled with trying to add a primary constructor, turn it into a data class, but each route I've hit a hurdle.

The things that stick out:

I'm calling validateMessageContents() in both constructors, but can't move it to init because the contents isn't yet available.
The second constructor, aside from validating the contents, is just two ugly assignment statements.
The two codepaths to either super() or super(id) make it difficult for one of these constructors to be made primary.

First, let's address your primary concern: the two constructors.

If the code to generate the UUID is not overly complicated, we can extract it and use it in our constructor as a default value (I don't like this because it adds code redundancy but I guess this is the only way to consolidate both constructors).
The same applies to the timestamp.
Thus we can use a primary constructor with the more specific super-constructor.

open class Message(
    id: String = UUID.randomUUID().toString(),
    val contents: String,
    val timestamp: Long = System.currentTimeMillis()
) : Unique(id) {

Here we combined both constructors and kept the order of the parameters.
Now we can use the init-block as well to validate the contents of the message:

init {
    validateMessageContents(contents)
}

That's the majority of things one can do to slim the class down.

Keep in mind that you need to specify a named parameter contents now because the first positional parameter id has a default value.

Message(contents = "test")

Another thing to do is to annotate your functions according to their behavior, so annotate validateMessageContents with @Throws.
I've also taken this opportunity to convert the two if-statements to a when-block. It's functionally the same (a string can't be empty and 1024 characters long at the same time).

@Throws(IllegalArgumentException::class)
    private fun validateMessageContents(contents: String) {
        when {
            contents.isEmpty() -> {
                throw IllegalArgumentException("Contents cannot be blank/empty")
            }
            contents.length > MAX_MESSAGE_LENGTH -> {
                throw IllegalArgumentException("Message too long - please limit to $MAX_MESSAGE_LENGTH characters.")
            }
        }
    }


We'll be counting stars Lately, I've been, I've been losing sleep
Dreaming about the things that we could be
But baby, I've been, I've been praying hard,
Said, no more counting dollars
We'll be counting stars, yeah we'll be counting stars
(One Republic - Counting Stars)

The 2nd Monitor is known to be a quite star-happy chat room, but exactly how many stars is there? And who are the most starred users? I decided to write a script to find out.

I chose to write this in Python because:

I've never used Python before
@200_success used it and it didn't look that hard
I found Beautiful Soup which seemed powerful and easy to use

The script performs a number of HTTP requests to the list of starred messages, keeps track of the numbers in a dict and also saves the pure HTML data to files (to make it easier to perform other calculations on the data in the future, and I had the opportunity to learn file I/O in Python).

To be safe, I included a little delay between some of the requests (Don't want to risk getting blocked by the system)

Unfortunately there's no way to see which user starred the most messages, but we all know who that is anyway...

The code:

from time import sleep
from bs4 import BeautifulSoup
from urllib import request
from collections import OrderedDict
import operator

room = 8595 # The 2nd Monitor (Code Review)
url = 'http://chat.stackexchange.com/rooms/info/{0}/the-2nd-monitor/?tab=stars&page={1}'
pages = 125

def write_files(filename, content):
    with open(filename, 'w', encoding = 'utf-8') as f:
        f.write(content)

def fetch_soup(room, page):
    resource = request.urlopen(url.format(room, page))
    content = resource.read().decode('utf-8')
    mysoup = BeautifulSoup(content)
    return mysoup

allstars = {}
def add_stars(message):
    message_soup = BeautifulSoup(str(message))
    stars = message_soup.select('.times').pop().string
    who = message_soup.select(".username a").pop().string

    # If there is only one star, the `.times` span item does not contain anything
    if stars == None:
        stars = 1

    if who in allstars:
        allstars[who] += int(stars)
    else:
        allstars[who] = int(stars)

for page in range(1, pages):
    print("Fetching page {0}".format(page))
    soup = fetch_soup(room, page)
    all_messages = soup.find_all(attrs={'class': 'monologue'})
    for message in all_messages:
        add_stars(message)
    write_files("{0}-page-{1}".format(room, page), soup.prettify())
    if page % 5 == 0:
        sleep(3)

# Create a sorted list from the dict with items sorted by value (number of stars)
sorted_stars = sorted(allstars.items(), key=lambda x:x[1])

for user in sorted_stars:
    print(user)

The results, you ask? Well, here they are: (Spoiler warning!) (I'm only showing those who have \$> 50\$ stars here, to keep the list shorter)

   ('apieceoffruit', 73)
 ('ChrisW', 85)
 ('Edward', 86)
 ('Yuushi', 93)
 ('Marc-Andre', 98)
 ('nhgrif', 112)
 ('amon', 119)
 ('James Khoury', 126)
 ('Nobody', 148)
 ('Jerry Coffin', 150)
 ('BenVlodgi', 160)
 ('Donald.McLean', 174)
 ('konijn', 184)
 ('200_success', 209)
 ('Vogel612', 220)
 ('kleinfreund', 229)
 ('Corbin', 233)
 ('Morwenn', 253)
 ('skiwi', 407)
 ('lol.upvote', 416)
 ('syb0rg', 475)
 ('Malachi', 534)
 ('retailcoder', 749)
 ("Mat's Mug", 931)
 ('Simon André Forsberg', 1079)
 ('Jamal', 1170)
 ('The Mug with many names', 2096) (Mat's Mug, retailcoder and lol.upvote is the same user)
 ('rolfl', 2115)

It feels strange to do .pop() to fetch data from the select soup, is there another approach available here? However, as this is the first time I'm using Python, any comments are welcome.

That's a beautiful excuse for writing code, and the final product is quite nice as well.

★ First of all, congrats to your Java-ridden mind for not forcing classes into Python where they aren't needed.

★ You import but do not use OrderedDict and operator. Remove unused imports.

★ It is customary to write your code in a way that allows it to be used as either a module, or a script. For this, the if __name__ == '__main__' trick is used:

if __name__ == '__main__':
    # executed only if used as a script

★ You declare a few variables like room, url, and pages up front. This hinders code reuse:

The room variable should not be declared up front as a global variable, but in your main section. From there, it can be passed down through all functions.
The url specifically but unnecessarily mentions the-2nd-monitor. This isn't harmful, but unnecessary as only the ID is relevant. Furthermore, url is an awfully short name for such a large scope. Something like star_url_pattern would be better – except that global “constants” should be named all-uppercase:

STAR_URL_PATTERN = 'http://chat.stackexchange.com/rooms/info/{0}/?tab=stars&page={1}'

Reserve plural names for collections. pages should rather be page_count. But wait – why are we hardcoding this rather than fetching it from the page itself? Just follow the rel="next" links until you reach the end.

★ That last idea could be implemented with a generator function. A Python generator function is similar to a simple iterator. It can yield elements, or return when exhausted. We could build such a generator function that yields a beautiful soup object for each page, and takes care of fetching the next one. As a sketch:

from urllib.parse import urljoin

def walk_pages(start_url):
    current_page = start_url
    while True:
        content = ... # fetch the current_page
        soup = BeautifulSoup(content)
        yield soup
        # find the next page
        next_link = soup.find('a', {'rel': 'next'})
        if next_link is None:
            return
        # urljoin takes care of resolving the relative URL
        current_page = urljoin(current_page, next_link.['href'])

★ Please don't use urllib.request. That library has a horrible interface and is more or less broken by design. You might notice that the .read() method returns raw bytes, rather than using the charset from the Content-Type header to automatically decode the content. This is useful when handling binary data, but a HTML page is text. Instead of hardcoding the encoding utf-8 (which isn't even the default encoding for HTML), we could use a better library like requests. Then:

import requests
response = requests.get(current_page)
response.raise_for_status()  # throw an error (only for 4xx or 5xx responses)
content = response.text  # transparently decodes the content

★ Your allstars variable should not only be named something like all_stars (notice the separation of words via an underscore), but also not be a global variable. Consider passing it in as a parameter to add_stars, or wrapping this dictionary in an object where add_stars would be a method.

★ I don't quite understand why you write each page to a file. I suspect this was intended as a debugging help, but it doesn't add any value to a user of that script. Instead of cluttering the current working directory, make this behavior optional.

★ Do not compare to None via the == operator – this is for general comparison. To test for identity, use the is operator: if stars is None. Sometimes, it is preferable to rely on the boolean overload of an object. For example, arrays are considered falsey if they are empty.

Talking is easy,
coding is hard.
Is this refactoring
equally bad?

import time
from bs4 import BeautifulSoup
import requests
from urllib.parse import urljoin

STAR_URL_TEMPLATE = 'http://chat.{0}/rooms/info/{1}/?tab=stars'

def star_pages(start_url):
    current_page = start_url
    while True:
        print("GET {}".format(current_page))
        response = requests.get(current_page)
        response.raise_for_status()
        soup = BeautifulSoup(response.text)
        yield soup
        # find the next page
        next_link = soup.find('a', {'rel': 'next'})
        if next_link is None:
            return
        # urljoin takes care of resolving the relative URL
        current_page = urljoin(current_page, next_link['href'])

def star_count(room_id, site='stackexchange.com'):
    stars = {}
    for page in star_pages(STAR_URL_TEMPLATE.format(site, room_id)):
        for message in page.find_all(attrs={'class': 'monologue'}):
            author = message.find(attrs={'class': 'username'}).string

            star_count = message.find(attrs={'class': 'times'}).string
            if star_count is None:
                star_count = 1

            if author not in stars:
                stars[author] = 0
            stars[author] += int(star_count)

        # be nice to the server, and wait after each page
        time.sleep(1)
    return stars

if __name__ == '__main__':
    the_2nd_monitor_id = 8595
    stars = star_count(the_2nd_monitor_id)
    # print out the stars in descending order
    for author, count in sorted(stars.items(), key=lambda pair: pair[1], reverse=True):
        print("{}: {}".format(author, count))


Loading CSV into MySQL using OOP PHP I need to make a simple CSV->MySQL script in OOP and I was wondering if there's anything I am doing wrong in a object-oriented sense.

I have a file name db.php where I connect to MySQL via MySQLi and then I have another class where I load a csv file into MySQL like this:

class csv extends Database {

        public $file = null;

   function __construct() {

            if(count($_SERVER["argv"]) > 1){
                $this->file = $_SERVER["argv"][1];
            }else{
                $this->file = "stock.csv";
            }

            $this->db = $db;

        }

        function saveCSVtoDB(){
            //opens the file
            $fp = fopen($this->file, "r");

            $db = Database::getInstance();
            $mysqli = $db->getConnection();

            $result = $mysqli->query("LOAD DATA LOCAL INFILE 'file.csv'
                                           IGNORE INTO TABLE products
                                               CHARACTER SET UTF8
                                        FIELDS TERMINATED BY ','
                                         LINES TERMINATED BY '\n'
                                              IGNORE 1 LINES
                                              (code, name, desc, stock, price, @avai)
                                                         SET available = IF(@avai LIKE '%yes%', 1, 0)
                                               ");

            if($result){
                echo "success";
            }else{
                die("error");
            }

        }
}

So I would recommend to not extend the CSV parser from the Database class.
Instead The CSV parser should get the Database per dependency injection through
the constructor. Also I would not recommend to access gobal vars like $_SERVER
in a class. Here an example how you could do that:

class CSVParser
{
    protected $file;
    protected $database;

    public function __construct($file, Database $db)
    {
        $this->file = $file;
        $this->db = $db;
    }

    public function saveCSVtoDB()
    {
        // @TODO import CSV to DB
    }
}

And you could use the class with:

$file = isset($_SERVER["argv"][1]) ? $_SERVER["argv"][1] : "stock.csv";

$parser = new CSVParser($file, Database::getInstance());
$parser->saveCSVtoDB();


Given a string, print another one without a given char My aim is to write a C program (I must create a function) that, given a string and a character, returns another string without the given char value.

For example: function("Hello World!", 'l'); /* Prints "Heo Word!" */

So I've programmed a lot with Java and I am used to programming but I am not used to C and in particular to pointers. I managed to do it with a single difference, this is the code:

#include <stdio.h>

#define STR_LEN 32

char *stringChop(char *s1, char *s2, char c);

int main() {
    char string[STR_LEN - 1] = "Esame di fondamenti";
    char stringChopped[STR_LEN - 1];
    stringChop(string, stringChopped, 'm');
    printf("%s\n\n", stringChopped);
    return 0;
}

char *stringChop(char *s1, char *s2, char c) {
    char *p = s1;
    char *q = s2;
    while (p < &s1[STR_LEN]) {
        if (*p != c) {
            *q = *p;
            p++;
            q++;
        } else {
            p++;
        }
    }
    return s2;
}

As you can see, in the function stringChop I've put 2 strings because I didn't know how to return one, so instead I used a pointer to one string initialised in the main() function and edited it with the use of pointers.
Now, my question is: How can I achieve the same thing with a function that accepts only one string and only a char value?

Passing strings and buffers in C is a pain.  For this problem, it would be idiomatic to write a function that accepts an in-out parameter — basically a mutable string.  It works well in this case because you know that the result will never be longer than the original string.

char *deleteChars(char *s, char c) {
    char *out = s;
    for (char *in = s; *in; in++) {
        if (*in != c) {
            *out++ = *in;
        }
    }
    *out = '\0';
    return s;
}

If the caller wants to obtain the result as a separate string, then let the caller duplicate the string before calling deleteChars().


SQL database query wrapper How can I write a better database query wrapper?

//code for connection
exports.connect = function(callback) {
    client.connect(function(err, conInfo){
        if (err) callback(err);
        conInfo = "Connection at " + client.host + ":" + client.port + "/"
            + client.database + " used by " + client.user;
        callback(null, conInfo);
    });
};

//for queries that do not return realation
exports.runQuery = function(query_str, columns, params, callback) {
    var sqlQuery = query_str + " " + columns;
    client.query(sqlQuery, params, function(err) {
        console.log("NEW QUERY____________________");
        console.log(sqlQuery);
        console.log(params);
        if (err) throw err;
        callback(null);
    });
};

//for select queries
exports.executeQuery = function(query_str, callback) {
    var query = client.query(query_str, function(err){
        console.log("NEW QUERY____________________");
        console.log(query_str);
        if (err) throw err;
        query.on("row", function(row, result) {
            result.addRow(row);
        });
        query.on("end", function (result) {
            callback(null, result.rows);
        });
    });
};

I don't do JS, even less node.js, but from the looks of it you seem to be taking in a string for your query, which means client code can do something like "select x, y, sum(z) from mytable where x = " + someValue + " group by x, y" - which is prone to SQL-injection: your client code needs to remember that.

If your executeQuery also took parameters, I think it would be clearer that when you need parameters, they should be passed as arguments to that function, not concatenated into the query string.


Split redirecting to multiple files in bash A function mycommand which runs command and:

Gives me three log-files which are:

*.stdout.log: everything from stdout
*.stderr.log: everything from stderr
*.full.log: everything (i.e., both stderr and stdout)

Prints all output to screen

The idea is that I can quickly see through the errors, skim for additional info in stdout. But in some cases both (stdout and stderr) messages should be seen as one as it gives context, thus I want a third file to do that. For real-time overview I obviously want to see it on my screen too.

This works but it is rather clumsy and doesn't look nice. I'm unsure whether this is the right way to do it or I should improve the code.

The snippet is from my *.*shrc.

mycommand () { command "$@" \
          > >(tee command_"$@"_$(date +%F_%T).stdout.log command_"$@"_$(date +%F_%T).full.log) \
          2> >(tee command_"$@"_$(date +%F).stderr.log command_"$@"_$(date +%F_%T).full.log >&2 ) } \

There are a number of things here which concern me.

First up, the stderr log file does not have the time on the file name (missing _%T). This is a classic copy-paste+partial-fix issue, you copied the same code to multiple places, then needed to fix it, but you only fixed some of them.

The solution to that is to extract the code to just one place, and reuse that:

local basename=command_"$@"_$(date +%F_T)
tee ${basename}.stderr.log

Now, basename is reusable, and if you want to change all three (stderr, stdout, full), then you can change them in just one place.

Your code does not handle complex commands gracefully. What if the command is ls -laR /etc .... how will it save away to log files called:

command_ls -laR /etc_2015-09-22_10:25:30.stdout.log

You need to rationalize that. I started with the spaces and slashes first... using a bash regex/replace for [ \/] to be replaced with an _ underscore.

local command="$@"
local base=command_${command//[ \/]/_}_$(date +%F_%T)

Next up, I am concerned that you have two tee processes both writing to the same "full" file. The one that starts second will overwrite what the first one started with, but I suspect that they will subsequently "merge" the results. It would be better to start a clean file, and then have both tee processes append to the existing file.

I messed around with your function, and came up with:

threepipe () {
    local command="$@"
    local base=rcommand_${command//[ \/]/_}_$(date +%F_%T)
    local both=$base.both.log
    rm -f $both
    touch $both
    $command 2> >(tee -a $both | tee "$base.stderr.log" >&2) | tee -a $both | tee "$base.stdout.log"
}

Note that only the stderr needs to be handled by the anonymous fifo. The redirect back to stderr keeps it out of the way of the rest of the stdout. Also, note the use of the -a flag for tee.


Retry loop for asynchronous HTTP requests I've got a method that needs to PUT data to a web API. Sometimes the connection fails, so I needed a way to do retries, but if the retries fail, I still need to capture the exception and re-throw it.

I've got something that I "believe" is working, but I've got a stupid throw at the end. Is there a better, more concise way to do this when working with async tasks?

private const int TotalNumberOfAttempts = 10;

public static async Task<HttpResponseMessage> PutWithRetriesAsync(string url,
    HttpContent content,
    AuthenticationHeaderValue authenticationHeaderValue,
    MediaTypeWithQualityHeaderValue mediaTypeWithQualityHeaderValue)
{
    var numberOfAttempts = 0;
    ExceptionDispatchInfo capturedException;

    do
    {
        try
        {
            return await PutAsync(url, content, authenticationHeaderValue, mediaTypeWithQualityHeaderValue);
        }
        catch (AggregateException ex)
        {
            capturedException = ExceptionDispatchInfo.Capture(ex);
            numberOfAttempts++;
        }
    } while (numberOfAttempts < TotalNumberOfAttempts);

    if (capturedException != null)
    {
        capturedException.Throw();
    }
    throw new Exception("That will never be thrown");
}

I would rewrite your code into a while (true) loop, that can only be exited using the return in your try or using a throw; inside a condition in your catch:

while (true)
{
    try
    {
        return await PutAsync(url, content, authenticationHeaderValue, mediaTypeWithQualityHeaderValue);
    }
    catch
    {
        numberOfAttempts++;
        if (numberOfAttempts >= TotalNumberOfAttempts)
            throw;
    }
}

This way, you don't need ExceptionDispatchInfo or the useless  (but required by the compiler) throw at the end.

I also changed catch (AggregateException ex) to catch all exceptions, because await usually doesn't throw AggregateException (unlike task.Wait() or task.Result).


Iterate through a list of data and add it to nested models I have to go through files with metadata and I am unhappy with the iteration and how I have build it due to the fact, that I have a modeltree with lists. So basically I iterate through filesOfFolder and pick each file and then I am adding the metadata. In the else I am checking, if the fileName has the same beginning then the fileNameBefore, because then it is a child of the last file, and it needs a different documentOrderNumber.
The modeltree is like and it cannot get changed:
Mainmodel
   Documents (public MainDocumentModel[] and AttDocumentModel[])
       MainDocumentModel
           FileContent
       AttDocumentModel
           FileContent

So as of this tree I did it so and I am totally unhappy with that:
string fileNameBefore = "";
model.Documentlist = new Documents
{
    Attdocument = new AttDocumentModel[filesOfFolder.Count() + 1]
};
int i = 0;
int j = 0;
int k = 0;

string mainPath = System.Web.HttpContext.Current.ApplicationInstance.Server.MapPath("~/App_Data/" + model.DocId + "/");
string directoryAttPath = mainPath + "Att/";
Directory.CreateDirectory(directoryAttPath);

foreach (var file in filesOfFolder)
{
    if (file.f_name.Contains("Maindocument") || file.f_name.Contains("Wrapdocument"))
    {
        FileContent fContent = new FileContent();
        fContent.FileType = "PDF";
        fContent.FileName = "Maindocument.pdf";

        MainDocumentModel mainDoc = new MainDocumentModel();
        mainDoc.MainDoc = new List<FileContent>();
        mainDoc.Message = "";
        mainDoc.Status = true;
        mainDoc.MainDoc.Add(fContent);

        model.Documentlist.Maindocument = new MainDocumentModel[1];
        model.Documentlist.Maindocument[0] = mainDoc;
        string mainDocumentPath = mainPath + fContent.FileName;
        System.IO.File.WriteAllBytes(mainDocumentPath, file.f_file);
    }
    else
    {
        if (!(fileNameBefore.Split(' ')[0] == file.f_name.Split(' ')[0]))
        {
            j = 0;
            i++;
        }
        else
        {
            j++;
        }
        FileContent attachment = new FileContent();
        string documentOrderNumber = GenerateOrderNumberForRis(i, j, fileNameBefore);
        attachment.FileName = documentOrderNumber + ".pdf";
        attachment.FileType = "PDF";

        AttDocumentModel attDoc = new AttDocumentModel();
        attDoc.AttDoc = new List<FileContent>();
        attDoc.Message = "";
        attDoc.Status = true;
        attDoc.ShowName = file.f_subject;
        attDoc.AttDoc.Add(attachment);

        model.Documentlist.Attdocument[k] = attDoc;
        System.IO.File.WriteAllBytes(directoryAttPath + documentOrderNumber + ".pdf", file.f_file);
        k++;
        fileNameBefore = file.f_name;
    }
}

Here are the models:
public class MainModel
{
    ....
    [XmlIgnore]
    public MainDocumentModel Maindocument { get; set; }

    [XmlIgnore]
    public AttDocumentModel Attdocument { get; set; }

    [XmlElement(ElementName = "documentlist")]
    public Documents Documentlist  { get; set; }
}

[XmlRoot(elementName: "maindocument")]
public class MainDocumentModel
{
    [XmlIgnore]
    public String Message { get; set; }

    [XmlIgnore]
    public Boolean Status { get; set; }

    [XmlElement("content")]
    public List<FileContent> MainDoc { get; set; }
}

[XmlRoot(elementName: "attacheddocument")]
public class AttDocumentModel
{
    [XmlIgnore]
    public String Message { get; set; }

    [XmlIgnore]
    public Boolean Status { get; set; }

    [XmlElement("showname")]
    public string ShowName { get; set; }

    [XmlElement("content")]
    public List<FileContent> AttDoc { get; set; }
}

public class FileContent
{
    [XmlElement("dataformat")]
    public string FileType { get; set; }

    [XmlElement("uri")]
    public string FileName { get; set; }
}

Some quick remarks.

Don't pointlessly abbreviate: fContent isn't a clear name, for instance.

i, j and k are okay names for variables in a for-loop, but here they really should have meaningful names.

Avoid adding words like "list" to names of collections. Especially when Documentlist isn't a collection, but the oddly named Documents. A class with a plural name? Which is then initialized by setting the badly named property Attdocument?

I'm assuming that you cannot change the property name f_name?

Are you certain that .Contains("Maindocument") will always work? Or should it be case insensitive?

In several places you seem to use string concatenation to create directory paths. Avoid this and instead use Path.Combine().

When you construct the MainDocumentModel, try to keep it succinct. (I have omitted Message = "" since IMHO it seems superfluous: why not simply leave it null?) Same for AttDocumentModel in the else, BTW.
  var mainDocumentModel = new MainDocumentModel
  {
      Status = true,
      MainDoc = new List<FileContent>{ fContent }
  };

I have trouble figuring out what if (!(fileNameBefore.Split(' ')[0] == file.f_name.Split(' ')[0])) is supposed to mean. As a matter of fact, that whole if...else seems to reference some kind of logic that really should be documented there with comments. It doesn't help that we don't have access to the code for GenerateOrderNumberForRis.

There are two places where you do documentOrderNumber + ".pdf".

Maindocument, Attdocument, ... are compound words and thus should be PascalCase.


Using mocking to test that a Django view renders as expected I have a Django view that ends with this...

return render(request, 'define/see_all.html', context)

I have mocked the render function and

@patch('define.views_func.render')
def test_get_define_see_all_complete(self, mock_render):
    """Testing that the queryset returned properly"""
    things = AllThings(group="Word Editors")
    things.model['definition'].is_complete = True
    things.model['definition'].save()
    t_url = reverse(
        'define:get',
        kwargs={'language': things.model['language']}) \
        + "?action=seeall&complete=True"
    req = self.rf.post(t_url, {"nada": "nada"})
    see_all(req, {'language': things.model['language']})
    self.assertTrue(len(mock_render.call_args[0][2]['words'].object_list) == 1)

I am basically just trying to see that the context that was passed to render contains the appropriate output from the view, but yuck the last line looks like garbage to me and I know I will not understand it if I come back to look at it later. Is there a better way to do this?

How about doing things in two steps?

render_context = mock_render.call_args[0][2]
self.assertTrue(len(render_context["words"].object_list) == 1

This way it should be reasonably easy to figure out what call_args[0][2] are.

Although, I'm not sure if it's a good idea to test things this way. There still could be a bug if the template's broken, and you're essentially testing the internal API (the context data structure) in the middle of the function. Unless that is what you want to test, of course.

Maybe it would be a better idea to factor the querying logic to its own function. I guess, one possible way of doing this would be probably a class-based view, like some ListView derivative.

This way you could test its get_queryset() (not the get_context_data) to return the dataset you expect.

And you can also use test Client to return a sanely-looking rendered template - parse it with something like lxml or BeautifulSoup, or just check response string with "Word Editors" in response (although it may make sense to use more "unique" strings, like UUIDs in this case).


Function for accessing regression output I just wrote this function for personal use only while working on a specific data project. I would be grateful for any feedback!

get.coef1 <- function(x) {
  x <- as.character(x)
  m.names <- paste("fit", x, letters[1:3], sep="")
  models3 <- matrix(numeric(12L), 3, 4)
  for (i in 1:3) {
    models3[i, ] <- summary(get(m.names[i]))$coefficients[2,]
  }
  return(models3)
}

Data to reproduce function above:

x <- rnorm(100)
y <- x^2/4
z <- rnorm(100)+y/100
w <- rnorm(100)

Edit1:
And three fitted regressions:

fit3a <- lm(y ~ x)
fit3b <- lm(y ~ x + z)
fit3c <- lm(y ~ x + z + w)

Whose coefficients on x I want to access viaget.coef1(3).

Your function has several problematic assumptions for the object names in the environment (get()) and the number of objects (fixed length in the for loop).

In base R it would be more idiomatic and flexible to use some function from the *apply family, which operate on a list of objects, e.g.

do.call(rbind, lapply(list(fit3a, fit3b, fit3c),
                      function(x) summary(x)$coefficients["x", ]))
     Estimate Std. Error t value Pr(>|t|)
[1,]  0.01512    0.02927  0.5164   0.6067
[2,]  0.01530    0.02985  0.5127   0.6093
[3,]  0.01732    0.02899  0.5975   0.5516

t(sapply(list(fit3a, fit3b, fit3c),
         function(x) summary(x)$coefficients["x", ]))
     Estimate Std. Error t value Pr(>|t|)
[1,]  0.01512    0.02927  0.5164   0.6067
[2,]  0.01530    0.02985  0.5127   0.6093
[3,]  0.01732    0.02899  0.5975   0.5516

Output of get.coef1(3) (with set.seed(1)):

        [,1]    [,2]   [,3]   [,4]
[1,] 0.01512 0.02927 0.5164 0.6067
[2,] 0.01530 0.02985 0.5127 0.6093
[3,] 0.01732 0.02899 0.5975 0.5516


Assembling very large files I have users uploading files sometimes as large as 100+ GB to a local web server.  The upload process works well and chunks come in at 50MB.  The problem seems to be after the file is uploaded, when the web server assembles the files and the server (24GB RAM), despite not showing any graphical signs of memory pressure, gets very sluggish.  I want to make sure my code isn't causing any unnecessary slow-downs.

Please suggest any more efficient way to do it.  If it is already ok, then I'll know to look at other aspects of the process.

# open the temp file to write into
with open(temp_filename, 'wb') as temp_file:
    # loop over the chunks
    for i in range(total_chunks):
        with open(os.path.join(get_chunk_filename(chunk_identifier, i + 1)), 'rb') as chunk_file:
            # write the chunk to the temp file
            temp_file.write(chunk_file.read())

I suggest you use existing library functions, e.g. shutil.copyfileobj to do the copying. Edit to clarify, as Gareth said:  Use shutil.copyfileobj(chunk_file, temp_file) instead of temp_file.write(chunk_file.read()).

Other than that (allocating and reading into Python objects via chunk_file.read()) there's no obvious flaws with the code, but I/O in Python is to be avoided on that scale anyway.  I'd even say you could try using a shell script with cat $FILES > $OUTPUT and it could perform better.


Java 8 unit test for third-party data feeds Use Case

I have a service that does processing of records from multiple third-party feeds. The steps are generally identical for each feed, but each feed is populated in a different location on an aggregate feed. I decided to write a generic test that would allow testing of all this processing to keep things DRY.

Test

The test basically tests whether the processor method in our service properly adapts and adds the incoming record to the appropriate aggregate list.

@Mock AggregateFeed dataFeed; // contains lists of POJOs
@Mock Exchange exchange; // camel mock.. not really important

@Mock SportsAdapter sportsAdapter; // used to convert string to POJO
@Mock BusinessAdapter businessAdapter; // used to convert string to POJO

FeedService fixture = new FeedService();

...

private <T1, T2> void shouldProcessRecord(BiConsumer<T1, Exchange> processor,
        T1 incomingRecord, T2 fakeAdaptee, Function<T1, T2> mockAdapter,
        Function<AggregateFeed, List<T2>> list) {
    List<T2> fakeList = Lists.newArrayList();

    when(mockAdapter.apply(incomingRecord)).thenReturn(fakeAdaptee);
    when(list.apply(dataFeed)).thenReturn(fakeList);

    processor.accept(incomingRecord, exchange);

    assertEquals(1, fakeList.size());
    assertSame(fakeAdaptee, fakeList.get(0));
}

@Test
public void shouldProcessSports() {
    String testRecordFeed = constructSportsRecordFeed();
    SportsRecord fakeSportsRecord = new SportsRecord();
    shouldProcessRecord(fixture::addSportsRecord, testRecordFeed,
        fakeSportsRecord, sportsAdapter::fromFeed, AggregateFeed::getSports);
}

@Test
public void shouldProcessBusiness() {
    String testRecordFeed = constructBusinessRecordFeed();
    BusinessRecord fakeBusinessRecord = new BusinessRecord();
    shouldProcessRecord(fixture::addBusinessRecord, testRecordFeed,
        fakeBusinessRecord, businessAdapter::fromFeed, AggregateFeed::getBusiness);
}

// builds up a csv delimited string that represents a sports article
private String constructSportsRecordFeed() {
    return new StringBuilder().append( ... ).toString();
}

// builds up a csv delimited string that represents a business article
private String constructBusinessRecordFeed() {
    return new StringBuilder().append( ... ).toString();
}

Concerns

The main problem I have doing things this way is that the signature just doesn't look natural. If another developer comes to create a test, they'll have to dig through the code to figure out how to use this test. On the other hand, it's much easier to do a copy and paste of the test and change around the variables, but that doesn't feel right either because it's not DRY.

Questions

How can this generic test be improved to be easier to use?
Since this is a test, should a copy-and-paste methodology be better since it's more direct on what the test does?
Any other advice/comments would be greatly appreciated.

I am only going to talk about the sports method, from there you should be able to get the info you need for the others as they are all basically the same.

DRY isn't a big priority

As I said in my earlier comment, DRY (don't repeat yourself) principle applies to production code more than unit test code. I think even in general it's easy to take this too far, readability is important, but that's off-topic. Your code however is tough to grasp.

To me, something like the below is more clear. I like to specify "given/when/then" in comments (although usually I do nothing more than // Given, etc.) but it's not necessary.

@Test
public void shouldProcessSports() {

    List<SportsRecord> fakeList = Lists.newArrayList();

    // Given an incoming feed of the sports variety
    String testRecordFeed = constructSportsRecordFeed();
    SportsRecord fakeSportsRecord = new SportsRecord();

    Function<AggregateFeed, List<SportsRecord>> list = AggregateFeed::getSports;

    when(mockAdapter.apply(testRecordFeed)).thenReturn(fakeSportsRecord);
    when(list.apply(dataFeed)).thenReturn(fakeList); // (*) See note below

    // When the processor accepts the feed and exchange
    processor.accept(testRecordFeed, exchange);

    // Then there should be one result and it should be the sports record
    assertEquals(1, fakeList.size());
    assertSame(fakeSportsRecord, fakeList.get(0));
}

Note: I don't have much experience with the :: operator, if AggregateFeed::getSports.apply(dataFeed) or something similar is valid I would prefer that instead of making the list variable.

DRY is still relevant though!

While writing tests like this takes up more space I think it's more important that the test be clear.

All that said I don't think DRY is completely useless in unit tests. Suppose you were doing more checks on fakeList, like making sure it's not null, contains a specified number of elements, etc. I would put all those assertions in a separate method. Basically the assertions that are tangential to what you are testing. For example, you're not specifically testing the list has one element above, the assertion you care most about is that the only element is fakeSportsRecord. You can't really do something like that so you have to assert that the first element is fakeSportsRecord. Anyways, I tend to put these tangential assertions in methods like this:

private static void verifyListExistsAndHasSingleElement(List<?> list) {
    assertNotNull(list);
    assertEquals(1, list.size());
}

Naming might be a little poor.

The name are mostly fine, but I felt like I needed to add more.

fakeList → resultList or outputList
testRecordFeed → incomingRecordFeed or givenRecordFeed
fakeSportsRecord → expectedSportsRecord
list, this is by far the worst. As shown in the note I don't know if this variable is even necessary. Even just function is better because it's a function, not a list.


Plugin for replacing text with HTML I've written a jQuery Plugin which searches text and replaces it with other text or HTML.

Now everything works quite well, but recently I came across this SO question where the OP could have use of my plugin. That's why I linked to the GitHub repo and asked him for critiques and/or contribution.

Soon after, he said that my plugin would have slow down his page by the factor 17 (I don't think this number is coming from the dev console though). Anyway, that comment made me think about how to make my plugin faster.

(function ($) {
    "use strict";
    $.fn.replaceMe = function (options) {
        var settings = $.extend({
            textToReplace: 'any_text',
            replaceWithText: 'othertext',
            globally: true,
            excludedTags: [
                'img',
                'span'
            ]
        }, options);
        $(this).contents().each(function (index, node) {
            var text,
                excludedTags;
            if(node.innerHTML !== undefined) {
                text = node.innerHTML;
            } else {
                text = node.nodeValue;
            }
            excludedTags = (node.localName === null || node.localName === undefined) ? '' : node.localName;
            if (text.match(new RegExp(settings.textToReplace, 'g')) && node.nodeType === 3 && !excludedTags.match(new RegExp(settings.excludedTags.join('|')))) {
                if(settings.globally) {
                    $(node).replaceWith(text.replace(new RegExp(settings.textToReplace, 'g'), settings.replaceWithText));
                } else {
                    $(node).replaceWith(text.replace(new RegExp(settings.textToReplace), settings.replaceWithText));
                }
            }
        });
    }
}(jQuery));

On the testing environment, the text to parse counts 169,310 chars or 27,391 words.

The Chrome console says that the script needs 42 ms to perform. Now I'm not sure if the script needs any improvement and if yes what could be done to make it faster.

I would think that a major time complexity comes from the usage of Regex which involves 'steps' in the processing of the expression, meaning the complex the statement, the more steps it can take.

Instead of using a Regex for when settings.globally is false, a normal String.replace can be done.

And instead of using a Regex for the global checks, I would suggest using a String.prototype.replaceAll polyfill like the following:

String.prototype.replaceAll = function(search, replace){
    var string = this;
    var index = 0;
    while ((index = string.indexOf(search)) !== -1) {
        string = string.replace(search, replace);
    }
    return string;
}

Which would change the settings.globally block into:

if(settings.globally) {
    $(node).replaceWith(text.replaceAll(settings.textToReplace, settings.replaceWithText));
} else {
    $(node).replaceWith(text.replace(settings.textToReplace, settings.replaceWithText));
}

I would also suggest removing jQuery from the solution as well. extend, $ (the element fetching), match and each can be polyfilled in, or simply converted to vanilla JavaScript.

function $(selector, el) {
    if (!el) { el = document; }
    return Array.prototype.slice.call(el.querySelectorAll(selector));
}
var extend = function(out) {
  out = out || {};
  for (var i = 1; i < arguments.length; i++) {
    if (!arguments[i])
      continue;
    for (var key in arguments[i]) {
      if (arguments[i].hasOwnProperty(key))
        out[key] = arguments[i][key];
    }
  }
  return out;
};
if (!String.prototype.includes) {
  String.prototype.includes = function() {'use strict';
    return String.prototype.indexOf.apply(this, arguments) !== -1;
  };
}

and for each: you can use Array.prototype.forEach (The polyfill is quite large, so I didn't include it) or a simple for loop.


Aggregating HTML files I am using PHP to keep my webpage organized.

The directory looks like this:

root/folder/file1
root/folder/file2
root/folder/file3
root/index.php

Each file is very simple html doc with content containing a title tag, which I use to create a nav bar.

I want index.php to aggregate all the content from file1, file2 and file3, in addition to creating a nav bar up at the top.

The code I have achieves this successfully, but I'm not sure if I'm doing this in a practical way.

$domain = "domain/"
$dir = "folder/";
$files = scandir($dir);

// loop over all the files, saving their contents and ids to arrays
$contents = Array();
$titles = Array();
$ids = Array();
for ($x = 0; $x < sizeof($files); $x++) {
  if ($files[$x] != "." && $files[$x] != "..") {
    $filename = $domain . $dir . $files[$x];
    $file = fopen($filename, "r");
    $data = file_get_contents($filename);
    array_push($contents, $data);

    $regex = '#<title>(.*?)<\/title>#';
    preg_match($regex, $data, $match);
    $title = $match[1];
    array_push($titles, $title);

    $regex2 = '#( |-|,)#';
    $id = preg_split($regex2, $title)[0];
    array_push($ids, $id);
  }
}

// the first loop sets the nav bar
echo "
";
echo "
";
for ($x = 0; $x < sizeof($contents); $x++) {
    echo "
" . $titles[$x] . "
";
}
echo "
";
echo "
";

// this second loop sets the contents
echo "
";
for ($x = 0; $x < sizeof($contents); $x++) {
    echo "
" . $contents[$x] . "
";
}
echo "
";

I like the idea of having all the content on one big page and the nav bar helps a lot when I'm viewing it on my phone.

Questions

Are there any obvious problems with the way I am aggregating my files?
Are there challenges I may encounter that I have not yet experienced?
Is there a generally accepted way of doing this while achieving the same result?

Basically you need to remove the unused fopen() call and change ponderous operators to handy ones as for some reason you made a peculiar choice in favor of the former, like for vs. foreach, array_push vs simple assignment, etc. Also consider a cleaner way to output HTML

<?php
$pattern = "domain/folder/*.*";
$files = glob($pattern);

// loop over all the files, saving their contents and ids to arrays

$contents = [];
$titles = [];
$ids = [];
foreach ($files as $filename)
{
    $data = file_get_contents($filename);
    $contents[] = $data;

    preg_match('#<title>(.*?)<\/title>#', $data, $match);
    $titles[] = $match[1];

    $ids[] = preg_split('#( |-|,)#', $match[1])[0];
}
?>

<?php foreach ($titles as $x => $title): ?>

<a href="#<?=$ids[$x]?>"><?=$title?>

<?php endforeach ?>

<?foreach ($contents as $x => $content): ?>
    <div id="<?=$ids[$x]?>"><?=$content?>

<?php endforeach ?>


Project Euler problem 92 (sequences formed by the sum of squares of digits) in Haskell I made the following program in Haskell to solve Project Euler's problem 92. The program works (i.e. the solution it yields is correct), but it's not exactly the fastest in the world.

The problem is:

  A number chain is created by continuously adding the square of the
  digits in a number to form a new number until it has been seen before.

  For example,

  44 → 32 → 13 → 10 → 1 → 1
  85 → 89 → 145 → 42 → 20 → 4 → 16 → 37 → 58 → 89

  Therefore any chain that arrives at 1 or 89 will become stuck in an
  endless loop. What is most amazing is that EVERY starting number will
  eventually arrive at 1 or 89.

  How many starting numbers below ten million will arrive at 89?

My code:

digits :: Int -> [Int]
digits 0 = []
digits x = digits (x `div` 10) ++ [x `mod` 10]

squareDigits :: [Int] -> [Int]
squareDigits x =  map (^2) x

sumSquareDigits :: Int -> Int
sumSquareDigits x = sum (squareDigits (digits x))

chainTo89 :: Int -> Bool
chainTo89 89 = True
chainTo89 1 = False
chainTo89 x = chainTo89 (sumSquareDigits x)

main :: IO()
main = print $ length $ filter (chainTo89) $ [1..9999999]

Any feedback on how this can improved, ranging from advice on good practices to very concrete algorithmic improvements, preferably without using too complicated Haskell concepts (I've only been programming in Haskell for three days) is more than welcome. If you are going to propose rather complex improvements (e.g. state monads?), then that's fine, so long as it's understandable to a beginner. :)

Note: I've implemented this in Python too, where I mapped known outcomes to a dictionary, which sped up the process considerably (i.e. the moment the chain function encounters a number already checked, it goes straight to the outcome without chaining further). However, I don't know how to do that in Haskell, if that is good practice in Haskell in the first place. This is exactly one of the reasons I asked this question in the first place; it feels as if it would be good to have some 'global' variable that can be used to map known values to, but that seems to go against Haskell's spirit.

Overall, your solution looks like a reasonable Haskell implementation of the question. Well done. I found some tiny bits to comment about anyway.

Separate algorithm from IO

Part of your algorithm is specified in the main function. To make it easier to play with it, it is better to do all computations in some other function, and only handle input/output in main. In your case:

solve :: Int -> Int
solve n = length $ filter chainTo89 $ [1..n - 1]

main = print (solve 1000000)

This allows us, for example, to call solve on various numbers in ghci, in tests, or in benchmarks.

benchmarks

Since you asked about algorithmic improvements, benchmarks are actually a good idea. To use the criterion benchmarking library, we add an import and replace the main function:

import Criterion.Main

-- code to benchmark

main = defaultMain
  [ bgroup "chain89"
    [ bench ("solve " ++ show n) $ whnf solve n
    | n <- [100000, 200000 .. 1000000]
    ]
  ]

This benchmarks the solve function on values below 1 million, in 100k steps. The question asks for 10 million, but i decided to run my benchmarks on the smaller inputs to get more rapid feedback. If you compile this to an executable (say, chain89), run it as chain89 --output chain89.html to produce a nich benchmark report in chain89.html. For your code as given, I get these numbers (in ms):

Looks more or less linear to me. Did you expect it to look linear? Maybe think about it a moment before reading on.

Linear?!

I didn't expect it to be linear, because we're duplicating the work in the recursive calls. You mentioned that caching the recursive results helped a lot, so I expected the complexity without caching to be clearly worse than linear, and to regain linear complexity by caching, as you would expect from a dynamic programming solution.

After seeing the benchmark results, I realized that for large numbers n, the result of sumSquareDigits n is always much smaller than n. To approximate the result after one step, consider this: if the input is n, it has k = log n / log 10 digits. In the worst case, these digits are all 9. Then the result is k * 9 * 9. For example, sumSquareDigits 999999 is only 486. So even if we multiply an already large input by 10, the result after the first step is not more than 81 bigger, which doesn't translate into too many additional recursive calls. There are additional recursive calls, so this is still slower than linear, but only very little, so we don't see it in the benchmark results.

Memoization for small inputs

We can memoize the result on small values, where small means that the values can occur after the first step. In other words, if we want to support inputs to n, we want to memoize the results for 1 to 81 * log n / log 10, because that's the biggest number we can see after the first step, see above.

Haskell supports an unusual approach to memoization: We can set up a lazy data structure which maps inputs to outputs, and in the initialization of each entry in the data structure, we access other entries. As long as we're never (directly or indirectly) accessing an entry from itself, the entries will be computed in an appropriate order.

Which data structure do we want here? We're going to use the consecutive numbers from 1 to (81 * log n / log 10) as inputs, so we probably want to use an array. We need to add an import, change chainTo89 to add the memoization and adapt solve to the new variant of chainTo89.

import Data.Array.IArray

-- other code here

makeChainTo89 :: Int -> (Int -> Bool)
makeChainTo89 n = compute where
  size = 81 * ceiling (log (fromIntegral n) / log 10)

  cache :: Array Int Bool
  cache = array (1, size) [(i, compute i) | i <- [1 .. size]]

  fetch x = cache ! x

  compute 89 = True
  compute 1 = False
  compute x = fetch (sumSquareDigits x)

solve :: Int -> Int
solve n = length $ filter chainTo89 $ [1 .. n - 1] where
  chainTo89 = makeChainTo89 n

I renamed chainTo89 to makeChainTo89 because makeChainTo89 n returns a function that computes the same values as the old chainTo89 when called with values that are less than or equal to n. The variable size holds the size of the memoization table and is computed as explained above. The cache is the memoization table itself. It contains an entry for all i between 1 and size, mapping i to compute i. The fetch function retrieves an entry from the memoization table, and the compute function does the actual work of computing the next element in the chain. Note that compute looks almost like the old chainTo89 except that it calls fetch instead of the calling itself recursively. But fetch will force a cache entry which contains a  call to compute in its thunk, so that the recursion structure is actually the same. Just when the same cache entry is forced again, the result is already there.

This only works if n is big enough (so we never move out of the table after moving into it) and if there are really no other cycles than the cycles involving 1 and 89. Here's how the benchmark results look like for this version of the code:

That's almost 60% faster.

Avoiding right-associative (++)

Your digits function appends an element to the right of an immutable linked list, which requires copying the whole list. Since the order of the digits is irrelevant for the reset of the algorithm, it would be better to prepend the found digit to the list. (Even if the order is relevant, it is better to first produce the reversed list of digits, and then reverse once).

This is the new digits function:

digits :: Int -> [Int]
digits 0 = []
digits x = x `mod` 10 : digits (x `div` 10)

It is another 35% faster:

More ideas

Some other low-level ideas for speeding this up further:

explore divMod or quotRem as an alternative to div and mod.
explore alternatives to (^ 2).

And one high-level idea:

avoid looping over all permutations of the same selection of digits.

Further improvements

Also check out Max Taldykin's answer for (impressive) additional improvements on top of what I propose in my answer.


Series of repetitive content-loading functions A friend helped me put the following code together, which is made up of a series of repetitive loadContent() functions. I've just shown the first two functions but they're all the same.

The code works fine but I'm wondering if anyone could give me a tip as to how I could reduce the bloat in this code and condense the functions somehow. I read somewhere that repetitive code usually means that the code can be simplified or shortened somehow. I'm no expert at jQuery or coding so I'm not sure where to start. I'm assuming that the code could be written a lot better but I might be wrong.

function loadContent1()
    {
            $('#content1').empty();
            if(typeof(counterArray[1]) != 'undefined' && counterArray[1] !=     null)clearTimeout(counterArray[1]);
            jQuery.ajax({
            type: "GET",
            url: "//cgi-bin/mggsn_ssr21.cgi",
            beforeSend:function(){
                    jQuery('#content1').append('');
                    jQuery('#refresh1text').html("
Loading...
");
            }
            })
            .done(function(data){
                    if(typeof(counterArray[1]) != 'undefined' && counterArray[1] != null)clearTimeout(counterArray[1]);
                    $('#content1').empty();
                    $('#content1').html(data);
                    //alert(data.slice(0.100));

                    countDownTimeArray[1]=countDownIntervalArray[1];
                    countDown(1);
            })
                    .fail(function(){
                    $('#content1').empty();
                    jQuery('#content1').html('
failed to load content
' + this.url);
            });
    }

    function loadContent2()
    {
            $('#content2').empty();
            if(typeof(counterArray[2]) != 'undefined' && counterArray[2] != null)clearTimeout(counterArray[2]);
            jQuery.ajax({
            type: "GET",
            url: "/cgi-bin/mtest2.cgi",
            beforeSend:function(){
                    jQuery('#content2').append('');
                    jQuery('#refresh2text').html("
Loading...
");
            }
            })
            .done(function(data){
                    if(typeof(counterArray[2]) != 'undefined' && counterArray[2] != null)clearTimeout(counterArray[2]);
                    $('#content2').empty();
                    $('#content2').html(data);
                    //alert(data.slice(0.100));

                    countDownTimeArray[1]=countDownIntervalArray[1];
                    countDown(2);
            })
                    .fail(function(){
                    $('#content2').empty();
                    jQuery('#content2').html('
failed to load content
' + this.url);
            });
    }

Joseph cleaned up the code a lot so let me point out a few things that you'll be able to use in your future code.

Consistent formatting greatly improves code readability and maintenance. There are many out there that are good starts, and I'm a fan of Google's language styles for the most part.
Since you're already depending on the definition of $, it's odd to mix it with jQuery in the same code. Pick one and stick with it.
To check that a value is neither null or undefined you can simply compare it to null using ==/!=. When you know the value cannot be some other falsy value (e.g. false, 0, or ''), you can simply use ! or the boolean context.

if (counterArray[1]) { ... }

DOM queries can be expensive on large pages, so it pays to save the results of a query if you're going to use it multiple times, and it cleans up the code. I like prefixing the name of a variable holding the results of a selector with a $ to make it clear that it's not a raw DOM element.

var $content = $('#content1');

Most jQuery methods return the selector itself, allowing you to chain multiple calls together.

$content.empty().html(data);

html completely replaces the contents of the element so there's no reason to call empty on it first. DOM manipulation causes a reflow; combine your manipulations when possible.


Basic user registration code My first attempt at a user registration code.

Login.php defines database log in variables (e.g. Database name, Table name, etc)
Header.php connects to the database.

I used ctype_alum for security of inputs. I also know that I shouldn't use mysql_error() for security issues but I've used the function as a placeholder.

Would love some feedback on possible improvements:

<?php
    require_once 'login.php';
    include 'header.php';

    echo <<<_REGFORM
    <form action="registration.php" method ="post">

    Register an account:
    Usernames and passwords must be at least 6 letters in length comprising of only alphanumeric characters.

    Username          <input type="text" name="regusername"/>
    Password          <input type="password" name="regpassword"/>
    Retype Password   <input type="password" name ="checkregpassword"/>
               <input type="submit" value="Register"/>
               <input type="hidden" name="clicked" value=yes/>

</form>
    _REGFORM;

    if ($_POST['clicked']){
        if ($_POST['regusername'] && $_POST['regpassword']){
            if (ctype_alnum($_POST['regpassword']) && ctype_alnum($_POST['regusername']) && (strlen($_POST['regusername']) > 5) && (strlen($_POST['regpassword']) > 5)){
                if ($_POST['regpassword'] === $_POST['checkregpassword']) {
                    // Insert code to check if the username exists
                    // Insert code to finally create the account!

                    $username = $_POST['regusername'];
                    $password = md5($_POST['regpassword']);

                    mysql_select_db($db_database) or die("Unable to select database: ".mysql_error());

                    $query = "SELECT * FROM userlogin WHERE username='$username'";
                    $result = mysql_query($query) or die("Database access failed: ".mysql_error);

                    if (!mysql_num_rows($result)) {
                        $query = "INSERT INTO userlogin VALUES('','$username','$password')";
                        $result = mysql_query($query) or die("Unable to create account: ".mysql_error());

                        echo "Thanks ".$username.", your account has been created!";
                    }
                    else echo"Username Unavailable, please try another username";

                }
                else echo "Your passwords do not match";
            }

            else echo "Username and password must be atleast 6 letters in length and letters a-z, A-Z and numbers 0-9 are accepted
";
        }
        else echo "Please fill in a valid username and password";
    }

    ?>

I think this is better than about 99% of first tries I've ever seen!

Anyway, I've tried my best to address as many things as I saw, whether major or extremely minor.  If you have any questions on any points or want to clarify or defend any claims, let me know and I will expand the answer.

Assuming array indexes exist

Any array that comes from user input should never be directly accessed.  $_POST['key'] is not safe since you can't know for sure whether or not the user supplied the $_POST['key'].

With arrays that can contain null values, you must use array_key_exists, but since input arrays like $_POST and $_GET will always have either strings or arrays, you can safely use isset.

The following is how I tend to access request driven arrays:

$username = (isset($_POST['username']) && is_string($_POST['username'])) ? $_POST['username'] : null;
if (strlen($username) < 5) {
    //username's must be >= 5 charactes
} else if (strlen($username > 32)) {
    //username's must be < 32 characters
}

//If you wanted to check if the user even provided a username:
if ($username === null) {
    //A username element of the form was not provided.  Depending on specific context, this may imply form tampering.
}

Note that the is_string check is necessary so that the user cannot maliciously pass an array to the script. If that were to happen, then the strlen call would complain about not expecting an array.  Unfortunately you have to be rather paranoid about anything coming from users.

There's a longer discussion of this here in the last section.

It's also worth noting that some people prefer a filter_input approach.

SQL injection

There is no SQL injection possible in your code, but that may be by accident.  When putting strings into queries, it's good practice to always run them through mysql_real_escape_string.  That way if you change your business rules in a way that does make SQL injection possible, you don't have to worry about the change cascading down to your queries and opening a security hole.

More information about SQL injection can be found in a post I wrote the other day, here.

(Sorry if I'm assuming that you don't know about SQL injection and you do.)

MySQL Extension

The mysql_* extension is quite dated, and PDO offers many advantages.

Offers a cleaner API
Offers better error handling (the ability to have failed queries throw exceptions is wonderful)
Handles transactions SQL-dialect agnostically (for the most part)
Different RDBMS can be changed out relatively easily (switching from mysql_* to PostreSQL is extremely painful.  Switching from PDO with MySQL to PDO with PostreSQL isn't pleasant, but is care is taken to stay as dialect-agnostic as possible, it's reasonably easy)
Prepared statements

You can find more information here under the PDO section.

pre can be used to arrange spacing, however you should be aware that it typically uses a fixed width font, and there's a few other quirks with it (you can probably style it to use a different font though).  I suppose there's technically nothing wrong with it, but pre doesn't strike me as a good long term method for arranging elements.

ctype_alnum for a password

Why restrict users to a a-zA-Z0-9 password?  I'm a firm believe that passwords should be allowed to be just about anything.  Some restrictions may be in order, such as ASCII only or something of that nature, but if a user wants his password to be this is my password, then why not?  You're hashing it anyway, so the length doesn't matter.  By the same logic, why not allow non alpha numeric characters?  It's not like HTTP or PHP will have any issues handling things like !@#$%^&*()|_+=- and so on.

list column fields in inserts

$query = "INSERT INTO userlogin VALUES('','$username','$password')";

Listing columns explicitly allows for better maintainability and clarity.  What if in the future you add a birthday column after the username column?  Suddenly you're trying to insert passwords into the birthday column.  Also, if you're going to put an auto incrementing column in a query, it should be null.

I would write the query like this:

$query = "INSERT INTO userlogin (username, password) VALUES ('$username','$password')";

userlogin

This sounds like a logging table to me.  This sounds like it contains records of users logging in, not of actual user records. I would consider renaming this table to something like users or user_accounts.

escape html where necessary

echo "Thanks ".$username.", your account has been created!";

Once again, since it's limited to alphanumeric, this is definitely harmless, however, preparing for the future never hurts.  If at some point you allow users to create names that contain &, this could output invalid html.

(Once again, sorry if I'm telling you something you already know.)

always use brackets

It's hard to see on a short script with fairly simple logic, but I'm a firm believer that braces should always be used.

if {} else { statement }

Looks clearer and is less error prone than:

if {} else statement

html element's attributes should always be in quotes

value=yes should be value="yes"

MD5

Hashing algorithms should be computationally expensive enough that a brute force attack is infeasbile.  Due to rainbow tables and other factors, a single MD5 run of an alphanumeric password is not considered secure.

Consider using something like bcrypt that is a bit heavier algorithm, or at least use something like 512 bit SHA repeated a thousand (or more!) times.

Let f be your hashing function.  Let S = the set of all possible passwords.  The absolute worst case time to brute force a password, p, such that p is in S is size of S * time for f(x)

If f = MD5 then the time is very small. MD5 and even SHA are considered fairly fast hashes.  If you use a an algorithm such as one of those, you should at least repeat it.  This way f becomes a much longer operation.

There's a few really good posts on StackOverflow about this, so I'll try to find some and link them.

This is a pretty good explanation (thanks to luiscubal for finding the link).

More graceful error handling

For the errors, I would probably handle them a bit better than the old or die construct that tends to go along with the mysql_ functions.

The idiom I would follow (though I would use PDO with exceptions), would be something like:

$query = "...";
$resource = mysql_query($query);
if (!$resource) {
    //log the error including all necessary details
    //output a fairly vague error the user
}

trigger_error can be useful for this.

Error handling suggestion

I like to validate everything separately and independently such that all errors can be output at once.  For example, if my username is only 4 characters and my passwords don't match, it would be nice to see both errors at once instead of having to fix one error, resubmit the form, fix the second error then resubmit the form.

This is by no means perfect, but the structure I typically use for error handling is:

$errors = array();
$username = $password = null;

if ($_SERVER['REQUEST_METHOD'] === "POST") {
    $username = ...;
    $password = ...;
    if (strlen($username) < 5) {
        $errors['username'][] = "Usernames must be 5 or more characters";
    }
    if (strlen($username) && !ctype_alnum($username)) {
        $errors['username'][] = "Usernames must be alphanumeric";
    }
    if (strlen($password) < 5) {
        $errors['password'][] = "Passwords must be 5 or more characters";
    }
    //More validations
    if (!count($errors)) {
        //No errors, so $username and $password are valid
        $insertionQuery = sprintf("INSERT INTO users (username, password) VALUES ('%s', '%s')", mysql_real_escape_string($username), mysql_real_escape_string($password));
        $insertionResult = mysql_query($insertionQuery);
        if (!$insertionResult) {
            trigger_error("MySQL query failed.  Query: [{$insertionQuery}].  Error: [" . mysql_error() . "]", E_USER_WARNING);
            $errors['db'][] = "There was an unexpected error. Please try again.";
        }
    }
}

Then, to render the errors, I typically just create a
 with each error as a
:

if (count($errors)) {
    echo '
';
    foreach ($errors as $element => $errs) {
        foreach ($errs as $err) {
            echo '
' . htmlspecialchars($err) . '
'
        }
    }
    echo '
';
}

The element name being the array key is useful because it means you can do things like style elements a certain way if they had an error:

<input type="text" name="username" value="<?php echo htmlspecialchars($username);"<?php if (array_key_exists('username', $errors)) { echo ' class="error"'; } ?>>

You could even take the idea farther and render errors around elements.

In general, it's a lot more flexible to aggregate the errors instead of immediately outputting them.

This philosophy touches into the area of MVC and separation of concerns.  MVC isn't particularly important for now, but the general idea of it (and in part SoC) is that each area of code should be concerned with one specific thing and nothing else.

A slight extension of this is the idea that logic and presentation should be separate.  Doing this allows many more degrees of freedom, and typically better code.  Consider for example if you wanted to have an AJAX interface for submitting the form, but still have an HTML fallback for users with JavaScript disabled.  If you were storing errors, you could do something simple like:

if (!empty($_POST['ajax'])) {
    echo json_encode(array('result' => 'errors', 'errors' => $errors));
    //set some kind of flag so that html output will not happen
    //(or, if it's a small script, I might just use `exit;`)
}

//Then later, if it's not AJAX
//render the errors as HTML inline in the form

If you intermix your logic and presentation code, the code is forever stuck rendering in exactly one way.  There's no way to reuse the same logic with a different presentation.  If they are separated, however, then the same logic can be used with infinite presentations.

Variable reuse

I'm definitely in the minority on this one, but I don't like to reuse variable names.  I would call the repeated query and result variables something like $existsQuery and $insertionQuery.  Nothing wrong at all with reusing the same name, just a personal preference (though the more descriptive a name, the better [within reasonable limits]).

header.php

This is not a very descriptive name.  I would consider calling it something like database.php or connect.php or something.  When I think of headers, I think content, not code.

Also, since the database connection is required for the page to function, require should be used instead of include.  A failed include will allow execution to continue, whereas a failed require will kill execution.

mysql_select_db

This should be with your database connection code.

login.php

No idea what "Login.php defines database log in numbers" means?  I'm a bit confused though about what this could be doing.  Also not sure why require_once instead of require?


Dataframe transformation to numpy ndarray takes ages to complete I would like to transform my dataframe into an array of fixed-sized chunks from each unique segment. Specifically, I would like to transform the df to a list of m arrays each sized (1,100,4). So at last, I would have an (m,1,100,4) array.
Since I require that the chunks be of fixed-size (1,100,4), and on splitting it is unlikely that each segment produce perfectly this size, the last rows of a segment are usually less, so should be zero-padded.
For this, I start be creating an array of this size, and populate it with all zeros. Then gradually fill in these values with df rows. This way, what's left at end of a particular segment is therefore zero-padded.
To do this, I use the function:
def transform(dataframe, chunk_size):

    grouped = dataframe.groupby('id')

    # initialize accumulators
    X, y = np.zeros([0, 1, chunk_size, 4]), np.zeros([0,])

    # loop over each group (df[df.id==1] and df[df.id==2])
    for _, group in grouped:

        inputs = group.loc[:, 'A':'D'].values
        label = group.loc[:, 'class'].values[0]

        # calculate number of splits
        N = (len(inputs)-1) // chunk_size

        if N > 0:
            inputs = np.array_split(
                 inputs, [chunk_size + (chunk_size*i) for i in range(N)])
        else:
            inputs = [inputs]

        # loop over splits
        for inpt in inputs:
            inpt = np.pad(
                inpt, [(0, chunk_size-len(inpt)),(0, 0)],
                mode='constant')
            # add each inputs split to accumulators
            X = np.concatenate([X, inpt[np.newaxis, np.newaxis]], axis=0)
            y = np.concatenate([y, label[np.newaxis]], axis=0)

    return X, y

This function does produce the intended ndarray. However, it is extremely slow. My df has over 21M rows, so the function takes more than 5hours to complete, this is crazy!
I am looking for a way to refactor this function for optimization.
Steps to reproduce the issue:
Generate a random large df:
import pandas as pd
import numpy as np
import time

df = pd.DataFrame(np.random.randn(3_000_000,4), columns=list('ABCD'))
df['class'] = np.random.randint(0, 5, df.shape[0])
df.shape
(3000000, 5)

df['id'] = df.index // 650 +1
df.head()

       A           B           C           D    class   id
0   -0.696659   -0.724940   0.494385    1.469749    2   1
1   -0.440400   0.744680    -0.684663   -1.962713   4   1
2   -1.207888   -1.003556   -0.926677   -1.455632   3   1
3   1.575943    -0.453352   -0.106494   0.351674    3   1
4   0.888164    0.675754    0.254067    -0.454150   3   1

Transform df to the required ndarray per unique segment.
start = time.time()
X,y = transform(df, 100)
end = time.time()

print(f"Execution time: {(end - start) / 60}")
Execution time: 6.169370893637339

For a 5M rows df this function takes more than 6mins to complete. In my case (>21M rows), it takes hours!!!
How do it write the function to improve speed? Maybe the notion of creating the accumulator is completely wrong.

Your code appears to be quadratic in the number of groups.  Each call to np.concatenate() allocates enough memory to hold the new array and then copies the data. The first group is copied the first time through the loop. Then the first and second groups on the second time. Then the first to third groups on the third time, etc.
To speed this up, keep a list of the groups and then call np.concatenate() just once on the list of groups.
Another observation is that the code splits a group into chunks only to reassemble them in the loop. The only differences are the group is padded to be a multiple of group_size and the shape of the array has changed. But those can be addressed without splitting and concatenating each group.
The Pandas documentation says to use DataFrame.to_numpy() method rather than .value.
Revised code:
def transform2(dataframe, chunk_size):

    parts_to_concat = []
    labels = []

    for _, id_group in dataframe.groupby('id'):

        group = id_group.loc[:, 'A':'D'].to_numpy()
        labels.append(id_group.loc[:, 'class'].iat[0])

        parts_to_concat.append(group)

        # add a zero-filled part to the list of parts to
        # effectively pad the group to be a multiple of chunk_size
        pad = chunk_size - len(group) % chunk_size
        if pad < chunk_size:
            parts_to_concat.append(np.zeros((pad, 4)))

    # reassemble the data and change it's shape to match
    # the output of the original code
    transformed_data = np.concatenate(parts_to_concat)
    transformed_data = .reshape(-1, 1, chunk_size, 4)

    labels = np.array(labels)

    return transformed_data, labels

On my laptop, the original code takes almost 12 minutes to run the sample dataframe. The new code takes less than one second--about a 700x speedup.


FIFO queue for a 2D array in ANSI C, version 2 This is a modification of my previous version.
I would like advice on improving a FIFO algorithm for a 2D array that I've developed in ANSI C.
I have written code that uses this algorithm for test purposes. Random numbers are put into an array until all elements are full. The program then asks the user what numbers need to go in to the 2D array in order to update it. The code then removes the oldest array, shifts all remaining arrays up by one, and then adds the new array (as input by the user).
I am asking for a review of the FIFO code, fifo_algorithm; the surrounding code is only for testing purposes. I would appreciate any suggestions that only use standard C.
code:
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>

void random_num_input(void);
void print_random_num_input(void);
void print_new_array(void);
void fifo_algorithm(void);

int user_num[5] = { 0 };
uint16_t big_array[10][5] = { 0 };
uint16_t big_array_copy[10][5] = { 0 };
uint16_t array_print[1] = { 0 };
uint8_t count = 0;
int main()
{
    random_num_input();
    printf("These are the original numbers in the big array:\n");
    print_random_num_input();
    while (count < 10)
    {
        printf("Please enter values to swap in:\n");
        for (int i = 0; i < 5; i++)
        {
            scanf("%d", &user_num[i]);
        }
           printf("\n\n");
           fifo_algorithm();
        count++;
    }
}
//stores random numbers between 0-255 into big_array and big_array_copy
void random_num_input(void)
{
    for (int j = 0; j < 10; j++)
    {
        for (int i = 0; i < 5; i++)
        {
            big_array[j][i] = rand() % 255;
        }
    }

}
//prints the random values within the big_array
void print_random_num_input(void)
{
    for (int j = 0; j < 10; j++)
    {
        for (int i = 0; i < 5; i++)
        {
            array_print[0] = big_array[j][i];
            printf("%d,", array_print[0]);
        }
        printf("\n");
    }
}
//This algorithm removes the oldest array in big_array_copy (element 9), shifts every
//element up by 1 and stores the user inputted array into element 0. Thus a fifo that updates
//the 2d array with the user input values every time
void fifo_algorithm(void)
{
    for (int j = 0; j < 10; j++)
    {
        for (int i = 0; i < 5; i++)
        {
            big_array_copy[j][i] = big_array[j][i];
        }
    }
    for (int k = 0; k < 5; k++)
    {
        big_array_copy[10 - 1][k] = 0;
    }
    for (int j = 9; j > 0; j--)
    {
        for (int i = 0; i < 5; i++)
        {
            big_array_copy[j][i] = big_array[j - 1][i];
        }
    }
    for (int i = 0; i < 5; i++)
    {
        big_array_copy[0][i] = user_num[i];
    }
    print_new_array();
    for (int j = 0; j < 10; j++)
    {
        for (int i = 0; i < 5; i++)
        {
            big_array[j][i] = big_array_copy[j][i];
        }
    }
}
//simply prints the new, updated values in big_array_copy
void print_new_array(void)
{
    printf("These array values have been swapped:\n ");
    for (int j = 0; j < 10; j++)
    {
        for (int i = 0; i < 5; i++)
        {
            array_print[0] = big_array_copy[j][i];
            printf("%d,", array_print[0]);
        }
        printf("\n");
    }
}

Thanks

Good formatting
Good use of (u)intN_t types
When printing, use a matching format specifier: printf("%" PRI16u , array_print[0]);
Avoid naked magic numbers
Define constants for flexibility and clarity
// int user_num[5] = { 0] };
//uint16_t big_array[10][5] = { 0 };

#define BIG_ARRAY_MAXJ 10
#define BIG_ARRAY_MAXI 5
int user_num[BIG_ARRAY_MAXI] = { 0 };
uint16_t big_array[BIG_ARRAY_MAXJ][BIG_ARRAY_MAXI] = { 0 };

Instead of 9, use BIG_ARRAY_MAXJ - 1, etc.
To address the "I could have put macros down to name 5 and 10 but I didn't see how that could be advantageous," comment: using named constants convey to others, including yourself at a later time, the relationship amongst the many constants and their individual meaning.
Global objects
Typically, it is far better to code for use of local objects than global ones.
Instead of
random_num_input(void)

Pass in a pointer to the array and its dimensions.  If VLA is supported:
random_num_input(int jn, int in, uint16_t a[j][i]) {
  for (int j = 0; j < jn; j++) {
    for (int i = 0; i < in; i++) {
        a[j][i] = rand() % 256;
    }
}

Array indexing
For big arrays, consider size_t instead of int to well handle large arrays.  size_t is the Goldilocks size for array sizing and indexing: not too small, not too big.
For big arrays, likely also need to move to allocated memory rather than fixed sizes.
Comment/code off by 1.
rand() % 255 forms values [0...254], not [0...255].
// stores random numbers between 0-255 into big_array ...
        big_array[j][i] = rand() % 255;

Consider
big_array[j][i] = rand() % 256;
// or
big_array[j][i] = (uint8_t) rand();

Copying arrays
With arrays of the same dimension and type, as here, could use memcpy().
// for (int j = 0; j < 10; j++) //
//    for (int i = 0; i < 5; i++) {
//        big_array_copy[j][i] = big_array[j][i];
//    }
//}

// if known to be non-overlapping
memcpy(big_array_copy, big_array, sizeof big_array_copy);
// else
memmove(big_array_copy, big_array, sizeof big_array_copy);

Minor: Tidier print
Consider not printing a trailing ','.  Little reason for a global array_print[].  Use the correct portable specifier
    char *sep = "";
    for (int i = 0; i < BIG_ARRAY_MAXI; i++) {
        printf("%s%" PRI16u , sep, big_array_copy[j][i]);
        sep = ",";
    }
    printf("\n");