Find a Value in a Container I have created a simple function to determine if a container contains a value. It allows for certain conditions to be set as well: where to to start searching for value, which way to search, whether to find the first or the last occurrence of the value, and whether or not to include the starting point. I want its return value to be either the index of the matching element, or -1 if no matches were found.

template<class Container, class T>
int Vec_Find(const Container& c, int size, const T& val, int pivot_idx = 0,
             bool find_first = true, bool ltr = true, bool include_pivot = true)
{
    if (find_first && ltr)
    {
        if (include_pivot) pivot_idx++;
        int i = std::distance(std::begin(c),
                    std::find(std::begin(c) + pivot_idx, std::end(c), val));
        if (i != size) return i;
    }
    else if (find_first && !ltr)
    {
        if (!include_pivot) pivot_idx--;
        int i = std::distance(std::find(std::rbegin(c) + (size - pivot_idx - 1),
                                        std::rend(c), val), std::rend(c) - 1);
        if (i != -1) return i;
    }
    else if (!find_first && ltr)
    {
        if (include_pivot && (c[pivot_idx] == val)) return pivot_idx;
        int i = std::distance(std::find(std::rbegin(c),
                            std::rend(c) - (pivot_idx + 1), val), std::rend(c) - 1);
        if (i != pivot_idx) return i;
    }
    else
    {
        if (include_pivot) pivot_idx++;
        int i = std::distance(std::begin(c),
                    std::find(std::begin(c), std::begin(c) + pivot_idx, val));
        if (i != pivot_idx) return i;
    }
    return -1;
}

I am a student to C++ and am looking for ways to code better, I appreciate all suggestions.

To Editors: I wasn't sure if I should leave my long lines on one line or fit to screen, please let me know if they should be on one line, and I will update it.

Parameters

As a starting point, I'd usually advise against passing bools as parameters--especially passing more than one bool to a particular function. I have better ways to spend my time than memorizing the meaning of:

foo(..., false, false, true);

vs.

foo(..., false, true, false);

I suppose I could barely see something like this if you were doing something like passing some bools, and it was putting them together into a bit set, so true, false, true basically just meant 101, but for something like this where each has a unique meaning, it seems unusually difficult to read.

Basic Utility

At least in my opinion, however, that nearly pales compared to the mere fact that I don't see much point in the function existing at all.

People who are accustomed to C++ are generally fairly accustomed to how the standard library works. Most of use would generally rather get an iterator than an index. If we want to search from end to beginning, passing rbegin() and rend() to find makes that intent fairly clear.

For example, if I want to search for the last item, starting 4 from the end, I'd have a much easier time understanding this:

auto p = std::find(foo.rbegin()+4, foo.rend(), value);

...rather than:

int p = Vec_find(foo, foo.size(), value, 4, false, true, true);

or:

int p = Vec_find(foo, foo.size(), value, 4, true, false, true);

[At least as I read things, those are basically the same--either find the last item, or find the first, starting from the end. Oh, and pardon me if I'd passed the wrong thing for the size parameter--I couldn't figure out exactly what it was supposed to do.]

Granted, iterators do tend to lead to fairly verbose code, but in this case, using your code can be even more verbose. I'm pretty sure when ranges become generally available (and sooner, for people who only need to target one compiler, or don't mind using Boost) quite a few people will welcome them with open arms. They can reduce verbosity quite a bit. Given a choice, however, between using this and using the standard library directly, I'd take the standard library without a second thought.


File Encryption using pynacl I need a rather simple file encryptor/decryptor in Python, after some research, I decided to use tye pynacl library reading the file in blocks, writing them back out, and then at the end using Blake2b to generate a signature for the file. Each file is encrypted with a unique key, which will be distributed along side the encrypted file, with the file key RSA encrypted using a pre-shared key pair, and that whole message signed with ECDSA to verify it came from me.

The encryption/decryption example code:

import base64
import struct
import nacl.secret
import nacl.utils
import nacl.hashlib
import nacl.hash

BUFFER_SIZE = 4 * (1024 * 1024)

def read_file_blocks(file, extra_bytes=0):
    while True:
        data = file.read(BUFFER_SIZE + extra_bytes)
        if not data:
            break
        yield data

def hmac_file(file, key):
    blake = nacl.hashlib.blake2b(key=key)
    with open(file, 'rb') as in_file:
        for block in read_file_blocks(in_file):
            blake.update(block)
    return blake.hexdigest()

def encrypt_archive(archive_name, encrypted_name):
    key = nacl.utils.random(nacl.secret.SecretBox.KEY_SIZE)
    #Use 4 bytes less than the nonce size to make room for the block counter
    nonce = nacl.utils.random(nacl.secret.SecretBox.NONCE_SIZE - 4)
    block_num = 0

    box = nacl.secret.SecretBox(key)
    with open(archive_name, 'rb') as in_file, open(encrypted_name, 'wb') as out_file:
        for data in read_file_blocks(in_file):
            #Append the block counter to the nonce, so each block has a unique nonce
            block_nonce = nonce + struct.pack(">I", block_num)
            block = box.encrypt(data, block_nonce)
            out_file.write(block.ciphertext)
            block_num += 1

    hmac_key = nacl.hash.sha256(key + nonce, encoder=nacl.encoding.RawEncoder)
    output = {}
    output['key'] = base64.b64encode(key + nonce)
    output['signature'] = hmac_file(encrypted_name, hmac_key)
    return output

def decrypt_archive(encrypted_name, archive_name, key_info):
    key_bytes = base64.b64decode(key_info['key'])

    key = key_bytes[:nacl.secret.SecretBox.KEY_SIZE]
    nonce = key_bytes[nacl.secret.SecretBox.KEY_SIZE:]

    extra_bytes = nacl.secret.SecretBox.MACBYTES
    hmac_key = nacl.hash.sha256(key_bytes, encoder=nacl.encoding.RawEncoder)
    hmac = hmac_file(encrypted_name, hmac_key)
    if hmac != key_info['signature']:
        print('hmac mismatch')
        return

    block_num = 0
    box = nacl.secret.SecretBox(key)
    with open(encrypted_name, 'rb') as in_file, open(archive_name, 'wb') as out_file:
        # nacl adds a MAC to each block, when reading the file in, this needs to be taken into account
        for data in read_file_blocks(in_file, extra_bytes=extra_bytes):
            block_nonce = nonce + struct.pack(">I", block_num)
            block = box.decrypt(data, block_nonce)
            out_file.write(block)
            block_num += 1

key_info = encrypt_archive("C:\\temp\\test.csv", "C:\\temp\\test.enc")
print(key_info)
decrypt_archive("C:\\temp\\test.enc", "C:\\temp\\test.enc.csv", key_info)

Outside of general mistakes, the two things I'm doing I'm not entirely sure are sound:

To keep the block nonces unique, I create a slightly smaller random list of bytes for the nonce than required, then when encrypting the blocks I append the block number, as a four byte integer to the nonce.
When generating the blake2b hash, for a key, I hash the file key and nonce. This seems somewhat useless overall, since if they have the key and nonce, they could just replace the file. Although, I can't really think of a better alternative that doesn't have similar weaknesses. Should I just ditch that bit, since NaCl does per-block MACs anyhow? (which I found out only after I wrote the hmac code)

Code / protocol comments under the code sections.

def read_file_blocks(file, extra_bytes=0):
    while True:
        data = file.read(BUFFER_SIZE + extra_bytes)

At least specify why the extra bytes are used and document what extra-bytes argument means.

def hmac_file(file, key):

Why would you go over the entire file and then dispose of the read bytes only to perform HMAC? You just processed all the blocks in memory! Why blake2b instead of one of the more common SHA-2 hashes?

#Use 4 bytes less than the nonce size to make room for the block counter

I'll bet that this isn't required; undoubtedly the counter is already included. Maybe you can point to a specific requirement to include a block counter every 4MiB?

hmac_key = nacl.hash.sha256(key + nonce, encoder=nacl.encoding.RawEncoder)

At least use HMAC instead of a hash to derive keys, the is a poor man's KDF.

output['key'] = base64.b64encode(key + nonce)

Sorry, output what? The key?

output['signature'] = hmac_file(encrypted_name, hmac_key)

CryptoBox uses an authenticated cipher. No need at all to HMAC it again.

  Algorithm details:
  Encryption:   Salsa20 stream cipher
  Authentication:   Poly1305 MAC


Printing alternating characters of a string using Java 8 streams Given a string, S, of length N that is indexed from 0 to N−1, I want to print its even-indexed and odd-indexed characters as 2 space-separated strings on a single line:

  Hacker -> Hce akr

My code works, but I feel it could have been done more elegantly and maybe in a single stream instead of in two... Any ideas how to improve this?

private static String reindex(String input) {
    StringBuilder result = new StringBuilder();

    IntStream.range(0, input.length())
             .filter(index -> index % 2 == 0)
             .forEach(index -> result.append(input.charAt(index)));

    result.append(" ");

    IntStream.range(0, input.length())
             .filter(index -> index % 2 == 1)
             .forEach(index -> result.append(input.charAt(index)));

    return result.toString();
}

The main issue with your code is that it operates with side-effects by leveraging the forEach operation. If you were to run your code in parallel, it would be broken and you wouldn't have the expected result. In this case, what you want is to use a mutable reduction approach, that is collect elements into a container.

All propose solutions hardcodes the fact that we reindex even and odd indexes. However, it would be possible to make them generic easily.

The first simple approach is to use 2 Stream pipeline (as you have done). The first one creates the first part of the String and the second one creates the second. Instead of using forEach to add elements to a StringBuilder, we collect the elements into a new StringBuilder. As such, each index is mapped to the character at that index and accumulated into a new StringBuilder with appendCodePoint. The 3 arguments to the collect call corresponds to:

the supplier of the mutable container, (StringBuilder::new);
the accumulator of each element into the container (appending the code point of the character);
a combiner combining two containers into one (used in parallel processing).

private static String reindex(String input) {
    StringBuilder result = new StringBuilder(input.length() + 1);
    result.append(filter(input, index -> index % 2 == 0));
    result.append(' ');
    result.append(filter(input, index -> index % 2 == 1));
    return result.toString();
}

private static StringBuilder filter(String input, IntPredicate predicate) {
    return IntStream.range(0, input.length())
             .filter(predicate)
             .map(input::charAt)
             .collect(StringBuilder::new, StringBuilder::appendCodePoint, StringBuilder::append);
}

The duplicate logic has been extracted to a utility method that filter the indexes matching the given predicate and collects the filtered characters into a StringBuilder.

But the issue is that it loops around the String two times when such an operation should be possible using a single traversal. This is a bit more complicated to do and I would first measure the performance of the first solution before doing this one: it is possible that your concerned String do not have a length big enough to warrant such an approach.

A possible solution is to collect the elements inside an array of StringBuilder. The array to append the current character to is determined by the result of i % 2: when i is even, this will be 0 and it will append it to the first array; when i is odd, it will append it to the second array.

private static String reindex(String input) {
    StringBuilder[] result =
        IntStream.range(0, input.length())
             .collect(
                () -> new StringBuilder[] { new StringBuilder(), new StringBuilder() },
                (b, i) -> b[i % 2].appendCodePoint(input.charAt(i)),
                (b1, b2) -> {
                    b1[0].append(b2[0]);
                    b1[1].append(b2[1]);
                }
             );

    return result[0] + " " + result[1];
}


speed up/optimize pandas code I am making a data-driven bond screening and I have as input a big dataset of 1526 columns and 2412 rows. For 10 columns it takes 2 minutes processing time at the moment, which is too much. The following function takes 90% of the time:

Input of the function is df: a pandas series, where the index is a time series and the first column has floats, like this:

https://imgur.com/a/3pQSQZC

def future_returns(df):
    grid_columns = np.arange(len(df))
    grid = pd.DataFrame(index=df.index, columns=grid_columns)

    # fill grid with copies of df, shifted 1 element forward for each column
    for no, idx in enumerate(grid.columns):
        grid.loc[:, idx] = df.shift(-no)

    # calculate future returns from every point in the index
    future_returns = grid.divide(grid.iloc[:, 0], axis=0) - 1

    return future_returns

code

Your code itself is clear, with and has only a few improvements

df

your parameter df expects actually a Series, and not a DataFrame, so I would rename this.

composition

now you first make an empty DataFrame and then change the values. More clear would be to generate it directly with the correct data:

def future_returns_2(data):
    grid = pd.DataFrame(
        index=data.index,
        columns=np.arange(len(data)),
        data=[data.shift(-i).values for i in range(len(data))],
    )
    return grid.divide(data, axis=0) - 1

Conveniently, this is also about faster

numpy

If you really want it a lot faster, you should stay in the numpy space for as long as possible, and only generate the DataFrame at the last possible time.

You can use numpy.roll

arr = data.values
result = np.array(
    [np.roll(arr, -i) for i in range(len(arr))],
    copy=False,
) / arr - 1

Since numpy.roll doesn't make the lower triangle of the result NaN, You should add this yourself:

mask = np.rot90(np.tri(l,), k=-1)
mask[np.where(1 - mask)] = np.nan
mask

array([[ 1.,  1.,  1.,  1.,  1.],
       [ 1.,  1.,  1.,  1., nan],
       [ 1.,  1.,  1., nan, nan],
       [ 1.,  1., nan, nan, nan],
       [ 1., nan, nan, nan, nan]])

Now you can deduct this mask instead of 1

def future_returns_numpy(data):
    arr = data.values
    l = len(arr)

    mask = np.rot90(np.tri(l), k=-1)
    mask[np.where(1 - mask)] = np.nan

    result = np.array(
        [np.roll(arr, -i) for i in range(l)],
        copy=False,
    ) / arr - mask

    return pd.DataFrame(data = result.T, index = data.index)

I find this code less clear than the pandas algorithm, but if speed is important, I would use this.

timings

For this dummy data

size=1000
np.random.seed(0)
data = pd.Series(
    np.random.random(size),
    index= pd.date_range(start='20190101', freq='1d', periods = size),
)

OP: 10.4 s ± 528 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
future_returns_2: 722 ms ± 29.8 ms per loop (mean ± std. dev. of 7 runs, 1 loop each)
future_returns_numpy: 79 ms ± 7.62 ms per loop (mean ± std. dev. of 7 runs, 10 loops each)


Burning logs - really simple text game What I have

I am a bit advanced PHP developer, but now I'm going to college where we are learning Ruby. We got the first homework and I did it. But I wanna make great code - so I looked for standards and found pretty much nothing. I only realized, that variables aren't named with camelCase but_like_this. I edited some things, but still I don't think this is great. Can someone familiar with Ruby tell me, how to improve this code? I am interested in details, because I wanna learn the best practice at the beginning.

Specific questions

Are functions without parameters bad? RubyMine highlights them and tells me "Parentheses around empty arguments list" but I am not sure, what does it mean.
How would be this code divided into more files (what would go where)? // It was task of the homework to make only one file

Long story short - the task

There is someone who lives in 1526. He has chunks of wood, which he can burn to make heat. Then he has logs, which he can split into chunks. Day by day he has four options.

Cut a tree (and get rand(2..4) logs )
Split rand(2..5) logs into chunks and get two chunks for each log
Go to sleep
Die

When he does something the day ends and he burns one chunk. Every day I must write what day it is and how many logs and chunks we have.

My code

def print_status()
  puts "It's day #{$day}. You have #{$logs} logs and #{$chunks} of chunks."
end

def print_action()
  puts "\nWhat will you do? \n 1. Cut a tree \n 2. Split logs into chunks \n 3. Go to sleep \n 4. Die and end the simulation"
  puts "\nYour choice:"
end

def print_delimiter()
  puts "\n------------------------------------------------------------"
end

def user_interaction()
  print_delimiter()
  print_status()
  print_action()
end

def end_day()
  puts "\nDay has passed and you burned a single chunk."
  $day += 1
  $chunks -= 1
end

$day = 1
$logs = 0
$chunks = 3

puts 'You are a logger in 1526. Try to survive.'

while $chunks > 0 do

  user_interaction()
  choice = gets.to_i

  case choice
    when 1
      chopped_trees = rand(2..4)

      $logs += chopped_trees

      puts "\nYou cut #{chopped_trees} trees, you got #{chopped_trees} logs from them."

      end_day()
    when 2
      if $logs == 0
        puts "\nYou have no logs to split."
        end_day()
        next
      end

      logs_to_chop = rand(2..5)

      if logs_to_chop > $logs
        logs_to_chop = $logs
      end

      $logs -= logs_to_chop
      $chunks += logs_to_chop*2

      puts "\nYou split #{logs_to_chop} logs to #{logs_to_chop*2} chunks."

      end_day()
      next
    when 3
      puts "\nYou go to sleep."
      end_day()
      next
    when 4
      puts "\nYou died."
      exit
    else
      puts "\nInvalid input"
      next
  end
end

puts 'You run out of chunks. You froze to death.'
exit

Conclusion

I hope you will tell me those little details to make my code best practice. And by the way - the homework is not in English, so it's not important to correct my grammar mistakes.

Ruby is a departure from PHP. It has a different syntax. Parentheses are often optionally when calling functions. Say you have the following function:

def add(x, y)
    x + y
end

The following are equivalent:

puts(add(3, 5))
puts add(3, 5)
puts add 3, 5

Similarly, these parentheses can be omitted, but only when the arguments are empty. Thus, the following are equivalent:

def hello()
    puts "world"
end
def foo
    puts "bar"
end

hello
foo()

So, as for one of your code examples:

def user_interaction()
  print_delimiter()
  print_status()
  print_action()
end

Can become:

def user_interaction
  print_delimiter
  print_status
  print_action
end

def print_status()
  puts "It's day #{$day}. You have #{$logs} logs and #{$chunks} of chunks."
end

Minor English quibble: it should be #{$chunks} chunks.

  if logs_to_chop > $logs
    logs_to_chop = $logs
  end

You can use a neat feature of ruby's syntax: a single-statement if block shaped like this:

if c
    d
end

Can become this:

d if c

Accordingly, this line could become:

logs_to_chop = $logs if logs_to_chop > $logs

However, this change is to your discretion.

You should move your global variables ($logs, $day, $chunks) to the beginning of your code.

$chunks += logs_to_chop*2

You should always put a space on either side of binary operators like * and +. So, it should become this:

$chunks += logs_to_chop * 2

Your code ends with an exit statement; this is unnecessary, since the program terminates at the end of the program anyhow.

Other than these suggestions, I think your code is really readable. Great job! In larger programs, you may wish to add comments. I don't think they're as crucial here, since the task itself is simple enough.


Async, callbacks, and closures After having spent a month or two trying to learn JavaScript, especially functional programming, async, and closures, I finally get it. But I don't know if I'm writing elegant code... Specifically, I'm not sure if I'm creating too many closures, because I don't entirely grasp when contexts remain and when they're marked for garbage collection.

You can probably tell from the example that I'm using node.js -- basically, this is just a piece of test code that connects to a database, gets 20,000 records asynchronously with individual SELECT queries (I'm just doing some profiling, don't worry), and manipulates the data. At the end, it tells me how long it took and closes the MySQL connection.

In order to get a final timestamp and close the connection, I needed something like the async library to keep track of all the various asynchronous functions that are being spawned. It takes all my queries as an array of functions (using a factory called makeQuery()) and then runs a callback to do the cleanup.

Questions:

Am I creating a huge amount of contexts by creating and returning the function doQuery() which uses the argument id in the makeQuery() function, thereby causing 20,000 contexts to persist until async.parallel() is run?
Is the factory function makeQuery() a good and tidy way to create this array of doQuery() functions? Can you suggest any better way to do it?

var mysql = require('mysql');
var radix64 = require('./lib/radix64');
var async = require('async');

var client = mysql.createClient({user: 'root', password: '12345678'});
client.query('USE nodetest');

var startTime = (new Date()).getTime(); // poor man's profiling
var queries = []; // this will be fed to async.parallel() later

var makeQuery = function makeQuery (id) { // factory function to create the queries
    return function doQuery (cb) {
        client.query('SELECT * FROM urls WHERE id='+id,
            function logResults (e, results, fields) {
                results = results[0];
                results.key = radix64.fromNumber(results.id); // yep, I'm converting the
                                                              // number into base-64 notation
                cb(); // if I wanted to do something useful at the end, I would have
                      // called cb(results) instead, which compiles an array of results
                      // to be accessed by the final callback
            }
        );
    };
};

for (i = 1001; i <= 20000; i ++) { // build the list of tasks to be done in parallel
    queries.push(makeQuery(i));
}

// run those tasks
async.parallel(queries, function finished () { // clean up and get the elapsed time
    console.log('done');
    client.end();
    console.log(((new Date()).getTime() - startTime) / 1000);
});

For those of you unfamiliar with async, it's a userland module built for node.js and the browser. Each function in the array that gets passed to async.parallel() is obliged to take a callback and then run that callback once it's done, in order to let async.parallel() know it's done.

var makeQuery = function makeQuery (id) {

There is no need to make makeQuery a local variable, just use a function declaration.

results = results[0];
results.key = radix64.fromNumber(results.id);

Your just augmenting the object, your not doing anything with it.

Other then that, your not creating new functions in a loop, your using a function constructor. This correct.

Your running all 19000 queries in parallel rather then in waterfall, which is also correct.

Edit:

Question 1:

You need to create 20000 contexts. Because there are 20000 values of i. Worrying about there being 20000 functions is a micro optimisation. v8 optimizes the hell out of your code.

It actually splits your functions into a hidden class seperate from the closure context, and in my memory you simply have 20000 values of i and one value for the function.

Just to remind you of rule 1.

  Never underestimate V8

Question 2:

makeQuery as a factory is the correct pattern to use.

The only other optimisation you can do is to write a real SQL query rather then 20000 dummy ones. But I'll ignore that because it's a dummy example.


HTML page listing product characteristics I am using below code in site. i am using lot of html "br" tags. Is this the proper way? or may i add css class for those lines instead of
 tags to give space between lines?

3D Sublimation Vacuum Heat Transfer Press Printer Machine ST-3040

It is a all-in-one machine that can transfer many sublimation blanks, mug, latte mug, plate, hat, t-shirt, iphone/ipad case, metal, glass, crystal, etc.

a) Feature / Intelligent:

1. Easy operation. You can only need to set the temperature and time.

2. Well-designed user interface.

Needs no operations other than placing the workpieces in and taking them out.

b) Multi-functional:

1.It can widely print on mug, glass, crystal, acrylic, plastic, wood, metal, stone, fabric, etc.

2.It can print on mugs of different shape and size with corresponding fixtures.

3.large print size; clear print effect; accurate color rendition; better performance than traditional heat transfer printers.

c) Delicate

1.Uneven dot surface enlarges the heating area.

2.Die casting aluminium alloy and far-infrared coating.

3.Even heating.

4.Clear print effect; accurate color rendition.

d) Concise

1.Made of flame-resistant heat insulating materials.

2.Safe, light, and streamline heat insulating strip.

3.Compact practical internal structure. Modern exterior.

e) Efficient

1.Large volume: 300 x 420 x 110 (mm).

2.Can print 12 mugs at one time.

f) Green&Compact

Built-in high performance acticarbon air filter.

Effectively filter the exhaust gas.

Long exhaust pipe.

108 flip design, easy operation, space saving.

Safe Software & hardware dual control of temperature. Each one will start automatically once the other one is failure.
15 minutes no operation off.

Specifications

Model: ST-3042

Voltage: 110 V /220 V 50 Hz /60 Hz

Power: 2800W

CBM: 0.18CBM

Printing Size: 300 x 420 x 110 mm

Vacuum Pump: 12 V/10 W

Max Vacuum: -640 mmHg

Pressure: -0.06 ~ 0.07 Pa

N.W: 24kg

Packing Size: 690 x 620 x 380 mm

HTML has elements that carry semantics. For example ol is an ordered list, ul an unordered one. Programs can use that meaning for ease of navigation, reader mode, better search results and so on. Certain elements don't carry that semantic, and
 is one of them.

Let's have a look at one of your snippets to see what one can improve.

c) Delicate

1.Uneven dot surface enlarges the heating area.

2.Die casting aluminium alloy and far-infrared coating.

3.Even heating.

4.Clear print effect; accurate color rendition.

First of all, that's invalid markup. The  and the
 tag intersect, which must not happen. Next, we have a list. It's apparently ordered, therefore we should use an
. And last, the
…
 looks like a headline, so it should be a h*. For this small snippet, we see that the following snippet contains all the semantics:

Delicate

Uneven dot surface enlarges the heating area.

Die casting aluminium alloy and far-infrared coating.

Even heating.

Clear print effect; accurate color rendition.

This is especially important to screen readers or other users that look for a structure. A blind person can decide to jump over a list; it's much more harder to jump over breaks in a paragraph.

So check what tags like
,
 and so on mean. Not only do they carry some kind of representation (this should look like a paragraph, this should look like a headline), but also meaningful semantics (this is a paragraph, this is a headline). Also, the code gets easier to read.

If you want to change they appearance, use CSS afterwards, e.g.

/* just an example to make it slightly more smoother */
body { line-height: 1.4; margin: auto; max-width: 80ex; }
h1 { font-size: 1.5em; }
h2 { font-size: 1.25em; }
li { padding-left: 1ex; }
li+li { margin-top: 0.4ex; }

3D Sublimation Vacuum Heat Transfer Press Printer Machine ST-3040

It is a all-in-one machine that can transfer many sublimation blanks, mug, latte mug, plate, hat, t-shirt, iphone/ipad case, metal, glass, crystal, etc.

Feature / Intelligent

Easy operation. You can only need to set the temperature and time.

Well-designed user interface.

Needs no operations other than placing the workpieces in and taking them out.

Multi-functional

It can widely print on mug, glass, crystal, acrylic, plastic, wood, metal, stone, fabric, etc.

It can print on mugs of different shape and size with corresponding fixtures.

large print size; clear print effect; accurate color rendition; better performance than traditional heat transfer printers.


Root-finding by iterated bisection Both of the following code give the same result.  But I'm not sure where should I put the raise statement.

def bisection(f, start, stop, eps = 1e-5, max_iteration = 100):
  for __ in range(max_iteration):
    half = start + (stop - start)/2
    if abs(f(half)) < eps or half - start < eps:
      return half
    if f(half) * f(start) > 0:
      start = half
    else:
      stop = half
  else:
    raise ValueError('Cannot find root in the given range')

Or

def bisection(f, start, stop, eps = 1e-5, max_iteration = 100):
  for __ in range(max_iteration):
    half = start + (stop - start)/2
    if abs(f(half)) < eps or half - start < eps:
      return half
    if f(half) * f(start) > 0:
      start = half
    else:
      stop = half

  raise ValueError('Cannot find root in the given range')

P.S.  I asked this question Stack Overflow and someone there suggested to ask it here.

Bug

I see no justification for or half - start < eps.  With that, you no longer guarantee that the return value is a root of f.


Sort and merge list based on another list Here I'm merging a list to end of another list after sorting it according to another list.This should sort processdetails list according to the order in routeplans list if any item not present in processdetails list is found it should keep the order and add it to processdetails list. Simply saying it just merging two lists based on another list.The below code works fine, I want to know whether there is cleaner way of doing this.if possible in linq.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace SortByList
{
public class OnGoingProcess
{
    public long Id { get; set; }

    public string Name { get; set; }
    public string Code { get; set; }
}

class Program
{
    static void Main(string[] args)
    {
        var processDetails = new List<OnGoingProcess>();
        processDetails.Add(new OnGoingProcess() { Id = 120, Name = "Process 29", Code = "T20" });
        processDetails.Add(new OnGoingProcess() { Id = 100, Name = "Process 2", Code = "A20" });
        processDetails.Add(new OnGoingProcess() { Id = 99, Name = "Process 3", Code = "S20"  });
        processDetails.Add(new OnGoingProcess() { Id = 85, Name = "Process 229", Code = "B20" });
        processDetails.Add(new OnGoingProcess() { Id = 220, Name = "Process 39", Code = "C20" });
        processDetails.Add(new OnGoingProcess() { Id = 15, Name = "Process 20", Code = "D20" });
        processDetails.Add(new OnGoingProcess() { Id = 101, Name = "Process 129", Code = "G20" });

        var routePlans = new List<OnGoingProcess>();
        routePlans.Add(new OnGoingProcess() { Id = 100, Name = "Process 2" });
        routePlans.Add(new OnGoingProcess() { Id = 120, Name = "Process 29" });
        routePlans.Add(new OnGoingProcess() { Id = 222, Name = "Process new" });
        routePlans.Add(new OnGoingProcess() { Id = 85, Name = "Process 229" });
        routePlans.Add(new OnGoingProcess() { Id = 101, Name = "Process 129" });

        var sortedPlans = new List<OnGoingProcess>();

        foreach (var plan in routePlans)
        {
            var selectedDetails = processDetails.Where(x => x.Id == plan.Id).ToList();

            if (selectedDetails.Any())
            {
                sortedPlans.AddRange(selectedDetails);
                processDetails.RemoveAll(x => x.Id == plan.Id);

            }
            else
            {
                sortedPlans.Add(plan);
            }
        }
        processDetails.AddRange(sortedPlans);

        foreach (var sorted in processDetails)
        {
            Console.WriteLine("Id : " + sorted.Id + " Name : " + sorted.Name + " Code : " + sorted.Code );
        }

        Console.ReadKey();

     //Id : 99 Name : Process 3 Code : S20
     //Id : 220 Name : Process 39 Code : C20
     //Id : 15 Name : Process 20 Code : D20
     //Id : 100 Name : Process 2 Code : A20
     //Id : 120 Name : Process 29 Code : T20
     //Id : 222 Name : Process new Code :
     //Id : 85 Name : Process 229 Code : B20
     //Id : 101 Name : Process 129 Code : G20

    }
}
}

Very poor problem statement.  Problem statement is based on your perceived solution so review is difficult.  I could only figure out what you are trying to do by reviewing the code.

Why long Id? If you have that many OnGoingProcess then will probably need a different approach.

Have proper constructors for OnGoingProcess and make Id read only.

Create a proper application and test.  Don't just jam everything in Main.

You are changing selectedDetails.   A calling procedure does not want an input changed.   Output a new List mergedDetails.

Proper problem statement
processDetails and sortedPlans are each List of OnGoingProcess identified by property Id
Need output in this order

processDetails not present in sortedPlans (in the order in    processDetails)
sortedPlans (in the order of sortedPlans)
if present in processDetails then processDetails
else then sorted plan

Putting those thoughts in action

public static void OnGoingProcessMergeTest()
{
    var processDetails = new List<OnGoingProcess>()
    { new OnGoingProcess(120, "Process 29", "T20"), new OnGoingProcess(100, "Process 2", "A20"), new OnGoingProcess(99, "Process 3", "S20"),
      new OnGoingProcess(85, "Process 229", "B20"), new OnGoingProcess(220, "Process 39", "C20"), new OnGoingProcess(15, "Process 20", "D20"),
      new OnGoingProcess(101, "Process 129", "G20")};

    var routePlans = new List<OnGoingProcess>()
    { new OnGoingProcess(100, "Process 2"), new OnGoingProcess(120,"Process 29"), new OnGoingProcess(222, "Process new"),
      new OnGoingProcess(85, "Process 229"), new OnGoingProcess(101, "Process 129")};

    List<OnGoingProcess> mergedDetails = OnGoingProcessMerge2(processDetails, routePlans);
    foreach (var sorted in mergedDetails)
    {
        Debug.WriteLine("Id : " + sorted.Id + " Name : " + sorted.Name + " Code : " + sorted.Code);
    }
    Debug.WriteLine("");
}
public static List<OnGoingProcess> OnGoingProcessMerge2(List<OnGoingProcess> processDetails, List<OnGoingProcess> routePlans)
{
    List<OnGoingProcess> onGoingProcessMerge = processDetails.Where(x => !(routePlans.Any(y => y.Id == x.Id))).ToList();
    foreach(OnGoingProcess routePlan in routePlans)
    {
        OnGoingProcess processDetail = processDetails.FirstOrDefault(x => x.Id == routePlan.Id);
        onGoingProcessMerge.Add(processDetail ?? routePlan);
    }
    return onGoingProcessMerge;
}


extract href from html I am building a Perl module which I am attempting to use as few non-core dependencies as possible. Should my following code fail, I would add a dependency on HTML::LinkExtor and be done with it, but I want to try. All I want is to extract the href= attributes from  tags. I do it using Text::Balanced which is core as of modern Perls and is installable for others. So yes, I know I should use a HTML library. That said, is this passably ok?

#!/usr/bin/env perl

use strict;
use warnings;

use Data::Dumper;

use Text::Balanced qw/extract_bracketed extract_delimited extract_multiple/;

my $html = q#Some link text stuff. And a little different link text.#;

my @tags = find_anchor_targets($html);

print Dumper \@tags;

sub find_anchor_targets {
  my $html = shift;

  my @tags = extract_multiple(
    $html,
    [ sub { extract_bracketed($_[0], '<>') } ],
    undef, 1
  );

  @tags =
    map { extract_href($_) }  # find related href=
    grep { /^)/) {
    return $1;
  } else {
    return ();
  }
}

This dumps

$VAR1 = [
          'link',
          'link2'
        ];

which is what one would expect.


Function for comparing floating point numbers I am trying to make a function for floating point numbers comparison.
The goal is to "evaluate" all operators in single function: >, >=, =, <=, <.
If X>Y, then F>0. If X==Y, F==0. If X<=Y, F<=0 etc (where F is returned value).
I took a piece of code from numeric limits. First thing I changed, in scaling of epsilon, fabs(x+y) to fabs(x)+fabs(y) (because if x=-y, we would be comparing diff to 0; which probably would work fine, but sounds kinda stupid). I've also seen an approach using max(fabs(x), fabs(y)), can we say that one of them is better than the others?
Third parameter, DecadesOfInAccuracy, means how many digits above epsilon() should be ignored.

#include <limits>
#include <iostream>

template<class T>
typename std::enable_if<!std::numeric_limits<T>::is_integer, int>::type
cmp(T x, T y, unsigned int doia = 0)
{
    std::cout << "X: " << x << std::endl;
    std::cout << "Y: " << y << std::endl;
    std::cout << "D: " << fabs(x - y) << std::endl;
    std::cout << "M: " << std::numeric_limits<double>::min() * pow(10, doia+1) << std::endl;
    std::cout << "E: " << std::numeric_limits<T>::epsilon() * (fabs(x) + fabs(y)) * pow(10, doia + 1) << std::endl;

    if (fabs(x - y) < std::numeric_limits<double>::min() * pow(10, doia + 1))
        return 0;
    bool ltoeq = (x - y) <= std::numeric_limits<T>::epsilon() * (fabs(x)+fabs(y)) * pow(10, doia + 1);
    bool gtoeq = (y - x) <= std::numeric_limits<T>::epsilon() * (fabs(x) + fabs(y)) * pow(10, doia + 1);
    std::cout << "L: " << ltoeq << std::endl;
    std::cout << "G: " << gtoeq << std::endl;
    return gtoeq - ltoeq;
}

int main()
{
    std::cout.precision(std::numeric_limits<double>::max_digits10 + 5);
    double x = 0.0001000000000000001;
    double y = 0.0001;

    int res = cmp(x, y, 0);
    std::cout << res << std::endl << std::endl;
    if (res > 0)
        std::cout << "X>Y" << std::endl;
    if (res >= 0)
        std::cout << "X>=Y" << std::endl;
    if (res == 0)
        std::cout << "X==Y" << std::endl;
    if (res <= 0)
        std::cout << "X<=Y" << std::endl;
    if (res < 0)
        std::cout << "X<Y" << std::endl;
    return 0;
}

It "correctly" returns equality for pairs like 0.0001000000000000001; 0.0001, 1.0-4*0.2; 0.2, 0; -0, 100000000000.0001; 100000000000.
Is there something I forgot to consider, or maybe something can be simplified? (of course except couts, which will be removed :P).
I am using VS2019 if that matters.

Is there something I forgot to consider

Overflow

fabs(x) + fabs(y) is prone to  overflow, even if mathematically epsilon() * (fabs(x) + fabs(y)) * pow(10, doia + 1) is representable as a T, resulting in incorrect results.

I'd expect cmp(T x, T y, unsigned int doia = 0) to work over the entire range of x,y.

// alternative
(fabs(x)/2 + fabs(y)/2) * (epsilon() * pow(10, doia + 1) * 2)

Not-a-number

The generation of gtoeq, ltoeq are 0 when either/both x or y are a not-a-number.  Thus code returns 0, incorrectly implying near equality.  Perhaps return something to indicate more that 3 conditions like a 4-bit (==,>,<,not comparable) or a double -1,0,1,NAN.

Precision

The + 1 in doia + 1 does not well allow code to just use epsilon, but minimally must use 10 * epsilon.  Consider dropping the + 1.

Rather than an integer indicating some power-of-ten * epsilon, consider simply using the desired precision or n * epsilon().  For me I'd rather use a power-of-2.


Await socket Response I wrote this to make the "callback hell" more manageable on the part of the coder when using Socket IO, so that there wasn't really any callback hell to go with, just a simple await.

This should work with any socket implementation, so long as it has socket.on, socket.emit, and socket.removeListener.

function emit_and_wait(socket, emit, data, wait_key, max_wait = 1000, error_on_timeout = true) {
    return new Promise(function(resolve, reject) {
        let responded = false,
             callback,
             removed = false;
        if(!socket) {
            reject("Socket supplied is not valid.")
        } else if(socket.disconnected) {
            reject("Socket is disconnected");
        }
        let timeout = setTimeout(function() {
            if(!responded) {
                if(error_on_timeout) {
                    reject("Socket timeout");
                } else {
                    resolve(null);
                }
                responded = true;
                if(!removed) {
                    socket.removeListener(wait_key, callback);
                    removed = true;
                }
            }
        }, max_wait)
        callback = function(data) {
            if(!removed) {
                socket.removeListener(wait_key, callback);
            }
            if(!responded) {
                resolve(data);
                clearTimeout(timeout);
            }
        }
        socket.on(wait_key, callback);
        socket.emit(emit, data)
    })
}

You have a few flags (removed, responded) which are almost equivalent and possibly useless. removeListener() won't cause any error if listener has been already removed then removed is unnecessary. After you called removeListener() your callback won't be called then also responded (which BTW is a misleading name) is unnecessary. Also, setTimeout() won't call its callback twice then again responded isn't necessary.

Use const instead of let whenever possible and declare your variables (usually one per line) when you initialize them. In (untested) code:

if (!socket) {
    reject("Socket supplied is not valid.");
    return;
}

if (socket.disconnected) {
    reject("Socket is disconnected");
    return;
}

const timeout = setTimeout(() => {
   socket.removeListener(wait_key, callback);

   if (error_on_timeout) {
       reject("Socket timeout");
   } else {
       resolve(null);
   }
}, max_wait)

const callback = (data) => {
    clearTimeout(timeout);
    socket.removeListener(wait_key, callback);
    resolve(data);
};

socket.on(wait_key, callback);
socket.emit(emit, data)

Few more notes: resolve() and reject() won't stop execution then you need to add the proper return. Use semicolon consistently: someone prefers to avoid semicolon as much as possible, pick a style and use it consistently.


Recursive matrix multiplication I've just written a recursive matrix multiplication and I want to know my mistakes or problems. Is there a better way doing this?

public static double[,] Multiply(double[,] a, double[,] b, int rowA,int colA,int rowB,int colB, double[,] c)
{
    double[,] result = new double[a.GetLength(0), b.GetLength(1)];

    if (c != null)
        result = c;

    result[rowA, colB] += a[rowA, colA] * b[rowB, colB];

    colA += 1;
    rowB += 1;
    if (colA >= a.GetLength(1))
    {
        colA = 0;
        rowB = 0;
        rowA += 1;
        if(rowA >= a.GetLength(0))
        {
            rowA = 0;
            colB += 1;
        }
    }

    if (colB >= b.GetLength(1))
        return result;

    return Multiply(a, b, rowA, colA, rowB, colB, result);
}

Edit:
The rowA, colA variables are the indices for the element of the first matrix which is multiplied by the [rowB, colB] element of the second matrix.
Then we increase colA and rowB so we can multiply the next elements of the matrices.

When the colA reaches the end of the matrix (it is obvious that rowB has also reached the end of the second matrix) we reset them to 0 and move to the next row of the first matrix. when we reach the last row of the first matrix we move to the next column of the second matrix by increasing colB by 1; We then set rowA to 0 so that the multiplication begins from the first row of the first matrix again.

Finally, if colB exceeds the end column of the second matrix we have finished multiplying and we return our result.

Edit 2:

public static double[,] Multiply(double[,] a, double[,] b, int rowA,int colA,int rowB,int colB, double[,] c)
{
    double[,] result;
    if (c != null)
        result = c;
    else
        result = new double[a.GetLength(0), b.GetLength(1)];

    result[rowA, colB] += a[rowA, colA] * b[rowB, colB];

    colA += 1;
    rowB += 1;
    if (colA >= a.GetLength(1))
    {
        colA = 0;
        rowB = 0;
        rowA += 1;
        if(rowA >= a.GetLength(0))
        {
            rowA = 0;
            colB += 1;
        }
    }

    if (colB >= b.GetLength(1))
        return result;

    return Multiply(a, b, rowA, colA, rowB, colB, result);
}

You're allocating a new result matrix every time, then almost always throwing it away.  Only allocate one if the passed in c matrix is null.

Since you're recursion comes at the end (tail recursion), this could be changed over from recursion to a loop.  If your matrix is too large, the recursion depth will be too deep and you'll run out of stack space.  This won't happen with a loop.


Python algorithm to return all two-tuple permutations of numbers up to n I have a Python function like so:
def get_values(n: int) -> list:
    """
    :param n: the maximum absolute value
    :return: sorted list of all the positive and negative permutations up to size n
    """
    if n < 0:
        return []

    if n == 0:
        return [(0, 0)]

    out = set()
    for i in range(0, n + 1):
        for j in range(0, i + 1):
            out.add((i, j))
            out.add((-i, j))
            out.add((i, -j))
            out.add((-i, -j))
    return sorted(out, key=lambda t: abs(t[0]))

The sorted at the end is important for my use case.
The issue is this uses a large amount of memory, presumably because it appends 4 values for every j. So as n becomes large, this set becomes very large.
I used a set rather than a list since I gathered it should take less memory by avoiding duplicates (I may be wrong :))
I would like to reduce the memory of this so can be large, up to 10,000 would be ideal but the bigger the better. Speed isn't really an issue here but faster is better!

I'd say your code is well-structured and easy to read. The approach however can be simplified using the itertools module from the standard library. itertools provides useful implementations for things like combinations, permutations, cartesian product and a lot more.
Here's the simplest and fastest approach I could come up with, that is equivalent to your implementation:
from itertools import product

def valid_tuple(number_pair: tuple) -> bool:
    a, b = number_pair
    return abs(b) <= abs(a)

def get_values_product(n: int) -> list:
    out = product(range(-n, n + 1), repeat=2)
    out = filter(valid_tuple, out)
    return sorted(out, key=lambda t: abs(t[0]))

It's usually faster to use functions that are built-in or included in standard library modules. itertools.product returns the cartesian product of input iterables, which is exactly what we want here. On my machine, this takes about half the time of your original implementation.

As you need sorted output I don't think there's a way to significantly reduce the space complexity. If you didn't need the output to be sorted, you could turn it into a generator, which is really memory-efficient.
Edit:
Here's how you can get the same desired sorted output while optimising space complexity:
from itertools import product, chain
from typing import Generator

def get_values_generator(n: int) -> Generator:
    if n < 0:
        return

    numbers = chain(*zip(range(n + 1), range(-1, -n - 1, -1)), [n])

    pairs = product(numbers, repeat=2)

    yield from filter(valid_tuple, pairs)

This works because we already pre-sort the list of numbers before calculating the cartesian product. This implementation returns a generator object, i.e. an iterator instead of a list. This means you might have to make some slight adjustments to the code that uses the return value of get_values.
Here's a comparison of the memory used by the return values of both approaches:
num = 50

sorted_list = get_values(num)
generator_obj = get_values_generator(num)

print(sys.getsizeof(sorted_list))
print(sys.getsizeof(generator_obj))

prints
> 85176
> 112

When changing num = 500, we get:
> 8448728
> 112

As you can see, the memory used by the generator object is constant at 112 bytes.
Here's some validation regarding the correctness of our result:
print("Same elements:\t", set(sorted_list) == set(generated_list))
print("Same length:\t", len(sorted_list) == len(generated_list))
print("No duplicates:\t", len(generated_list) == len(set(generated_list)))

print("Sorted:\t\t\t", all(abs(x[0]) <= abs(y[0]) for x, y in zip(generated_list, generated_list[1:])))

We get the same output for negative and positive numbers, as well as zero:
> Same elements:     True
> Same length:       True
> No duplicates:     True
> Sorted:            True


N Queens with Haskell I tried to solve the N Queens problem in Haskell using backtracking. Here is my approach:

type Solution = [Int]

extend :: Solution -> Int -> Maybe Solution
extend b new = go 0 b
  where
    n = length b
    go _ []  =  Just (b ++ [new])
    go i (x:xs)
      | x == new                      = Nothing
      | abs (i - n) == abs (x - new)  = Nothing
      | otherwise                     = go (i+1) xs

fullExtend :: Int -> Solution -> Maybe Solution
fullExtend n partialSolution
  | length partialSolution == n     = Just partialSolution
  | otherwise                       = foldr propose Nothing [0..(n-1)]
  where
    propose _ (Just xs) = Just xs
    propose x Nothing   = extend partialSolution x >>= fullExtend n

nQueens :: Int -> Maybe Solution
nQueens n = fullExtend n []

Can the propose function be expressed in a more elegant way? I am only happy to get one solution here, and shortcircuiting the evaluation is fine.

I also experimented a bit more to find out all the solutions. Here is how I did so:

extend' :: Solution -> Int -> [Solution]
extend' b new = go 0 b
  where
    n = length b
    go _ []  =   [b ++ [new]]
    go i (x:xs)
      | x == new                      = []
      | abs (i - n) == abs (x - new)  = []
      | otherwise                     = go (i+1) xs

fullExtend' :: Int -> Solution -> [Solution]
fullExtend' n partialSolution
  | length partialSolution == n     = [partialSolution]
  | otherwise                       = [0..(n-1)] >>= (\x -> extend' partialSolution x >>= fullExtend' n)

nQueens' :: Int -> [Solution]
nQueens' n = fullExtend' n []

General comments on coding style would also be helpful.

TL;DR:

Don't repeat yourself. If you notice that code is very similar, ask whether you can use that similarity.
Express functions with other functions.
Avoid length in recursive calls, as it will add another factor n to your time complexity

Overall, still well done.

Naming and (time) complexity

Let's start by looking at the types. Well, you have type signatures at every top-level binding, which is a great start. I think that the name Solution is slightly misleading, since you call the variable b for Board. But naming is hard, so let's focus on other parts.

extend :: Solution -> Int -> Maybe Solution
extend b new = go 0 b
  where
    n = length b                                       -- 1
    go _ []  =  Just (b ++ [new])                      -- 2
    go i (x:xs)
      | x == new                      = Nothing
      | abs (i - n) == abs (x - new)  = Nothing
      | otherwise                     = go (i+1) xs

extend has two small problems. First, xs ++ [e] (see 2 above) is linear (\$\mathcal O(n)\$), whereas e : xs is constant (\$\mathcal O(1)\$). If possible, we  should prefer the second variant. And length b is also linear (see 1 above). But if we fix 2 we don't need n anymore:

extend :: Solution -> Int -> Maybe Solution
extend bs new = go 1 bs
  where
    go _ [] = Just (new : bs)
    go i (x:xs)
      | x == new           = Nothing
      | i == abs (x - new) = Nothing
      | otherwise          = go (i + 1)

Note that I've renamed b to bs, since the suffix s indicates multiple elements.

Don't repeat yourself

Although yout extend and extend' differ, the logic in go is almost the same. That's a sign that you can re-use the code, so let's try that:

placeable :: Int -> Solution -> Bool
placeable n xs = null [ x | (i,x) <- zip [1..] xs, x == n || abs (x - n) == i]
-- same as
placeable' :: Int -> Solution -> Bool
placeable' n xs = go 1 xs
  where
    go _ []     = True
    go i (x:xs) = x /= n && abs (x - n) == i && go (i + 1) xs

Now we can simplify extend and extend':

extend :: Solution -> Int -> Maybe Solution
extend bs new = if placeable new bs
                  then Just (new : bs)
                  else Nothing

extend' :: Solution -> Int -> [Solution]
extend' bs new = if placeable new bs
                   then [new : bs]
                   else []

Feel free to use guard syntax instead.

Reuse functions

If you have a function listToMaybe :: [a] -> Maybe a, you don't need to write extend:

extend :: Solution-> Int -> Maybe Solution
extend bs new = listToMaybe (extend' bs new)

We can do the same for nQueens and get rid of extend completely:

nQueens :: Int -> Maybe Solution
nQueens n = listToMaybe (nQueens' n)

But you need to be lazy enough for that.

Use knowledge you already have

In fullExtend, you'll check whether you already reached the correct length:

fullExtend :: Int -> Solution -> Maybe Solution
fullExtend n partialSolution
  | length partialSolution == n     = Just partialSolution             -- here
  ...

However, that's again \$\mathcal O(n)\$. But we know the length of our current solution; almost. It's the number of recursive calls:

  ...
    propose x Nothing   = extend partialSolution x >>= fullExtend n  -- one element more

Also n does not change. This calls for a go-like function:

fullExtend :: Int -> Solution -> Maybe Solution
fullExtend n = go n
  where
    go 0 partialSolution = Just partialSolution
    go k partialSolution = foldr propose Nothing [0..(n-1)]
      where
        propose _ (Just xs) = Just xs
        propose x Nothing   = extend partialSolution x >>= go (k - 1)

We now count down from n to 0 in go. Whenever we reached 0, we have all n elements. This removes the need for length. At this point fullExtend is almost the same as nQueens, so we can get rid of it:

nQueens :: Int -> Maybe Solution
nQueens n = go n []
  where
    go 0 partialSolution = Just partialSolution
    go k partialSolution = foldr propose Nothing [0..(n-1)]
      where
        propose _ (Just xs) = Just xs
        propose x Nothing   = extend partialSolution x >>= go (k - 1)

We can use the same approach for fullExtend'. First, get rid of length:

fullExtend' :: Int -> Solution -> [Solution]
fullExtend' n partialSolution = go n partialSolution
  where
    go 0 xs = [xs]
    go k xs = [0..(n-1)] >>= (\x -> extend' partialSolution x >>= go (k - 1))

Then rewrite nQueens':

nQueens' :: Int -> [Solution]
nQueens' n = go n []
  where
    go 0 xs = [xs]
    go k xs = [0..(n-1)] >>= (\x -> extend' xs x >>= go (k - 1))

By the way, this looks a little bit nicer with do notation:

nQueens' :: Int -> [Solution]
nQueens' n = go n []
  where
    go 0 bs = [bs]
    go k bs = do
              x  <- [0..(n-1)]
              xs <- extend' bs x
              go (k - 1) xs

But that's up to you to decide.

Your propose question

  Can the propose function be expressed in a more elegant way?

You can use Maybe's Alternative instance:

propose x solution = solution <|> (extend partialSolution x >>= go (k  -1))

However, that's not really necessary if you rewrite nQueens

All at once

If we also inline extend, we end up with the following code for the list variant:

import Data.Maybe (listToMaybe)

type Board = [Int]

nQueens :: Int -> Maybe Board
nQueens = listToMaybe . nQueens'

nQueens' :: Int -> [Board]
nQueens' n = go n []
  where
    go 0 bs = [bs]
    go k bs = concatMap (go (k - 1)) [ x : bs | x <- [1..n], placeable x bs]

    placeable n xs = null [ i | (i,x) <- zip [1..] xs, n == x || abs (n - x) == i]

We could rewrite go above also as

    go k bs = do
              x <- [1..n]
              guard (placeable x bs)
              go (k - 1) (x : bs)

or

    go k bs = [1..n] >>= (\x -> guard (placeable x xs) >> go (k - 1) (x : bs)

but that's only personal preference, they all work the same.


Deciding whether an anchor should be updated A colleague and I were discussing the following code today and had a disagreement over the return false in the catch (MalformedURLException e) block.

For some background information this method is in a class which is going through legacy data to be migrated into a new system, parsing the text, finding all of the <a> tags and re-writing some of those links depending on what the link is to. This method is determining if the link should be updated or not based on some criteria.

private boolean shouldUpdateAnchor(final String link, String replaceFrom, String replaceInto) {
    if (link == null) {
        return false;
    }

    if (!link.contains(URL_NODEREF_QUERY_PARAM)) {
        return false;
    }

    if (!link.contains(replaceFrom) && !link.contains(replaceInto)) {
        return false;
    }

    final URL url;
    try {
        url = new URL(link);
    } catch (MalformedURLException e) {
        return false;
    }

    final String query = url.getQuery();
    final NodeRef documentNodeRef = getNodeRefFromQueryString(query);
    final QName type = nodeService.getType(documentNodeRef);
    if (type == null) {
        return false;
    }

    if (!type.equals(ContentModel.KnowledgeCentreDocument.QNAME)) {
        return false;
    }

    return true;
}

One of us said that using exceptions for control flow is an anti-pattern and the exception should be thrown/allowed to propagate up and handled elsewhere, while the other said that the error being thrown is a by-product of the API (this is Java code which doesn't have any of the tryParse() methods of C#) which needn't be thrown any further as it can be handled here.

I've read these related questions but I'm not entirely sure they're applicable here as they're quite general.

https://stackoverflow.com/questions/729379/why-not-use-exceptions-as-regular-flow-of-control
https://softwareengineering.stackexchange.com/questions/189222/are-exceptions-as-control-flow-considered-a-serious-antipattern-if-so-why

To provide some context, here are the two methods which are called immediately before this one:

public final String transformLinks(final String contentString, String replaceFrom, String replaceInto) {
    if (StringUtils.isBlank(contentString))
        return contentString;

    final Document document = Jsoup.parse(contentString);
    final Elements anchors = document.getElementsByTag("a");
    for (final Element anchor : anchors) {
        handleAnchor(anchor, replaceFrom, replaceInto);
    }
    return document.body().toString();
}

private void handleAnchor(final Element anchor, String replaceFrom, String replaceInto) {
    final String linkHref = anchor.attr("href");
    if (!shouldUpdateAnchor(linkHref, replaceFrom, replaceInto)) {
        return;
    }

    final String newHref = linkHref.replace(replaceFrom, replaceInto);
    anchor.attr("href", newHref);
}

About Exceptions
In general, exceptions should never be part of the anticipated program flow. Using exceptions to test conditions in the data, is wrong.
On the other hand, that is not what you are doing. You are taking an anchor/link from an HTML document, and I would expect that to always be a well-formed URL.
Additionally, a poorly-formed URL in the supplied context is not a program fault either, so propagating an exception is not useful. You will need to handle it somewhere else if you do.
My only suggestion is that you should be logging it somewhere... don't you need to know that you have HTML pages with broken links?
Concerns
You have some programming problems.... I am unable to tell if it is serious, it may be fine depending on your circumstances... consider:

if (!link.contains(replaceFrom) && !link.contains(replaceInto)) {
    return false;
}

... you return false if the link contains the replacement String, even if it also contains the to-replace string. Perhaps you should make the code just:
if (!link.contains(replaceFrom)) {
    return false;
}

As I say, there may be a business reason why you also need to check for the replaceInto, but, as it is at the moment, I believe you may be missing some replacements because the anchor contains both texts...
Style
You have the code:

final URL url;
try {
    url = new URL(link);
} catch (MalformedURLException e) {
    return false;
}

Code like this makes me itch ;-) , and not because of the return false, but because 'dangling declarations' of the url are a smell to me. Note that the type conditions can be simplified since they are boolean checks. Your try/catch code would be better written as:
try {
    final URL url = new URL(link);

    final String query = url.getQuery();
    final NodeRef documentNodeRef = getNodeRefFromQueryString(query);
    final QName type = nodeService.getType(documentNodeRef);

    return ContentModel.KnowledgeCentreDocument.QNAME.equals(type);

} catch (MalformedURLException e) {
    return false;
}

Note that the constant QNAME is not null, I assume, and that it's equals() method is presumably an equals-contract compatable implementation, which means it should return false for QNAME.equals(null);...


Implement Fibonacci range I am implementing a Fibonacci range in C++17 such that it supports

#include "fib.h"

#include <iostream>

int main() {

  for (int x : Fibonacci()) {
    std::cout << x << std::endl;

    if (x > 100000) {
      break;
    }
  }

  for (int x : Fibonacci(5)) {
    std::cout << x << std::endl;
  }

  for (int x : Fibonacci(3, 10)) {
    std::cout << x << std::endl;
  }

  return 0;
}

Here is how I defined the header

#ifndef FIB_H
#define FIB_H

#include <cstddef>
#include <iterator>

class Fibonacci {
 public:
  class iterator {
   public:
    using iterator_category = std::forward_iterator_tag;
    using value_type = int;
    using difference_type = int;
    using pointer = int*;
    using reference = int&;

   public:
    iterator() = default;

    iterator(int i, int a, int b);

    int operator*() const;

    iterator& operator++();

    bool operator!=(iterator const& other) const;
   private:
    int i_ {0};
    int a_ {0};
    int b_ {1};
  };

 public:
  Fibonacci();

  Fibonacci(int end);

  Fibonacci(int begin, int end);

  iterator begin() const;

  iterator end() const;

 private:
  iterator beginIt_;

  iterator endIt_;
};

#endif

And the source code

#include "fib.h"

#include <iterator>

Fibonacci::iterator::iterator(int i, int a, int b)
    : i_(i), a_(a), b_(b) {}

int Fibonacci::iterator::operator*() const {
  return b_;
}

Fibonacci::iterator& Fibonacci::iterator::operator++() {
  ++i_;
  b_ = b_ + a_;
  a_ = b_ - a_;
  return *this;
}

bool Fibonacci::iterator::operator!=(Fibonacci::iterator const& other) const {
  return i_ != other.i_;
}

Fibonacci::Fibonacci()
    : endIt_(-1, 0, 0) {}

Fibonacci::Fibonacci(int end)
    : endIt_(end, 0, 0) {}

Fibonacci::Fibonacci(int begin, int end)
    : beginIt_(std::next(Fibonacci::iterator(), begin)), endIt_(end, 0, 0) {}

Fibonacci::iterator Fibonacci::begin() const {
  return beginIt_;
}

Fibonacci::iterator Fibonacci::end() const {
  return endIt_;
}

I want to have a syntax similar to the standard library where the iterator of a class can be accessed using ::iterator, therefore I nested my iterator class into the main class itself. However, this makes the source code very verbose and results in very long function name. Is there a way to make this more readable? Furthermore, is there any new C++17 feature I should be aware of that can be applied here?

Overall, I think the code looks good.  Here are a few sugestions that may help you improve your code.

Check for overflow

As you know, terms of the Fibonacci sequence get very big very quickly.  For this reason, it's easy to overflow the numerical range.  I'd suggest addressing this by throwing an exception in iterator& operator++():

if (b_ < a_) {
  throw std::overflow_error("Exceeded range of underlying type");
}

Use an unsigned type

None of the terms of the Fibonacci sequence are negative, so I'd suggest that perhaps unsigned or even unsigned long might be more appropriate base types.

Use a template

It may make sense to turn this into a templated function to allow different types to be used as the base type.  This would be one way to extend the range, such as with the GNU Multiple Precision Arithmetic Library.

Consider a different operator++ implementation

The current implementation of the operator++ is this:

iterator& operator++() {
  ++i_;
  b_ = b_ + a_;
  a_ = b_ - a_;
  return *this;
}

I would suggest, especially if you decide to implement templates, that this might be more efficient:

iterator& operator++() {
  ++i_;
  std::swap(a_, b_);
  b_ += a_;
  if (b_ < a_) {
      throw std::overflow_error("Exceeded range of underlying type");
  }
  return *this;
}


Follow up to Pay Rate Calculator This is a Follow-up to Simple pay rate calculator

Here is what I came up with.  Hopefully if I have missed something that was said in answers on the previous question, we can still get those things pointed out.  I am hoping that I made the code cleaner.

Main Method

class MainClass
{
    public static void Main (string[] args)
    {
         //var payCheck = new PayRateCalculator("Hart",40,23);
        var payCheck = new PayCalculator();
        Console.Write ("Please provide the Employees Last Name >>");
        payCheck.lastName = Console.ReadLine();
        Console.Write ("How many hours has employee {0} worked? >>", payCheck.lastName);
        payCheck.hours = Convert.ToInt32 (Console.ReadLine());
        Console.Write ("What is employee {0}'s hourly wage? >>", payCheck.lastName);
        payCheck.payrate = Convert.ToInt32 (Console.ReadLine ());
        Console.WriteLine ("{0}'s Gross pay is ${1}", payCheck.lastName, payCheck.GrossTotal());
        Console.WriteLine ("{0}'s Net pay is ${1}", payCheck.lastName, payCheck.NetTotal());
    }
}

PayCalculator Class

class PayCalculator
{
    private const decimal _PayRateDefault = 8.25m;
    private decimal _payrate = _PayRateDefault;
    private decimal _withholdingRate = 0.20m;
    private decimal _hours;

    public decimal hours {
        get {
            if (hours < 1) {
                return 40;
            } else {
                return _hours;
            }
        }
        set {
            _hours = value;
        }
    }
    public decimal grossPay { get;set; }
    public decimal netPay { get; private set; }
    public decimal payrate {
        get {
            return _payrate;
        }
        set {
            if (value < 8.25){
                _payrate = 8.25m;
            } else {
                _payrate = value;
            }
        }
    }
    public decimal WithholdingRate{ get; set; }
    public string lastName { get; set; }

    public decimal GrossTotal ()
    {
        grossPay = hours * payrate;
        return grossPay;
    }

    public decimal NetTotal ()
    {
        grossPay = GrossTotal();
        netPay = grossPay - (grossPay * _withholdingRate);
        return netPay;
    }

    public PayCalculator (string lastName, decimal hours, decimal payRate)
    {
        this.hours = hours;
        this.payrate = payrate;
        this.lastName = lastName;
    }

    public PayCalculator ()
    {
    }
}

I am going to put the beginner tag on this again because I know that my coding skills are still not very good.

This will be the last review for this little bit of code.  I am going to see what else catches my eye in this book while I am working through it.

Well, you have started to use decimal instead of float and introduced a couple of named constants but pretty much every other point I stated in my answer to the other question still stands.

Naming conventions (PascalCase for properties)
You now have introduced a WithholdingRate property but that doesn't affect anything (the property is not used anywhere in the class)
Although you have introduced a named constant _PayRateDefault you are still using magic numbers in various places.
You have added some rules but they are implemented inconsistently - in hours you enforce the rule in the get while in payrate you do it in the set.
The way you have added the rules is simply bad - you silently coerce the values and the user of your class won't have a clue what's going on.

Assume this calculator is used to process the payroll in a company with lots of employees. Further assume that some of those are contractors which have not worked in the last week for whatever reason. Now you read in the work hours for each employee and everyone who hasn't worked at all (hours == 0) will get a full week payed - ouch.

If properties have restrictions on what range of values they should be in then enforce them in the set method by throwing an ArgumentException stating what was violated. It's better for your program to crash with an exception than to spit out bad data.

One of the few places where coercion of values makes sense is in user controls which force out of range values back into the range and then feed that back to the user (e.g. numeric up/down controls which a given min and max) but in most other cases it will do more harm than good.

Unfortunately your code has become somewhat more convoluted than the original.


Refactoring code to throw a single exception per function One of my functions must throws 2 exceptions, but I understand 1 exception per function is preferred (at least according to my client's Sonar ruleset).  Can you suggest how I might refactor the following code to enable 1 or no exceptions on these functions?

Worth noting is that I have a global exception handler which wraps this class, so any thrown exception will be caught there.

public ResponseEntity postFooRequest(
        @RequestBody String body,
        @RequestParam(value = "action") String action,
        @RequestParam(value = "fooNumber") String fooNumber) throws UnsupportedEncodingException, InterruptedException {
    String sanitizedBody = inputSanitizer.sanitizeBody(body);
    FooRequest fooRequest = new FooRequest(sanitizedBody, action, fooNumber);
    return processFooRequest(fooRequest);
}

private ResponseEntity processFooRequest(FooRequest fooRequest) throws InterruptedException, UnsupportedEncodingException {
    FooRequest fooRequestResult = fooRequestDelegate.processFooRequest(fooRequest);
    return new ResponseEntity<>(decodeResponse(fooRequestResult.getBody()), httpHeaders, HttpStatus.OK);
}

private String decodeResponse(String body) throws UnsupportedEncodingException {
    return java.net.URLDecoder.decode(body, "UTF-8");
}

UnsupportedEncodingException can easily be avoided by wrapping the call that might throw it:

try {
  return java.net.URLDecoder.decode(body, "UTF-8");
} catch (UnsupportedEncodingException ex) {
  throw new IllegalStateException(ex);
}

Indeed, "UTF-8" is one of most common encodings and that would be really surprising that such exception is thrown here. Semantically, it is close to an illegal state of the application: one would hardly imagine that it cannot support this encoding nowadays.

Since the chained IllegalStateException is a RuntimeException, you will be able to remove it from the signatures of the methods.


CRUD app for managing customers I am practicing using the Prototype pattern and to do so, I am building a simple CRUD app that allows me to view, add, and delete customers.

Here is a JsFiddle for you to see what I've got so far.

// Create customer 'class' and store state here
var Customer = function(el) {
    this.el = document.getElementById(el);
    this.first_name = $("#input_first_name").val();
    this.last_name = $("#input_last_name").val();
    this.name = ""

}

var customers = [];

// In the Customer prototype store methods
Customer.prototype = {

    // utility to log customer array
    logCustomers: function() {
        console.log(customers);
    },

    fullName: function() {
        this.name = this.first_name + " " + this.last_name;
        return this.name;

    },

    addCustomer: function() {
        // clear out the current html to make room for the append later
        this.el.innerHTML = "";

        // create new customer instance
        var cust = new Customer();

        // add customer to the array
        customers.push(cust);

        // iterate through the array and add each customer to a row in the table
        $.each(customers, function() {
            $("#el").append("
" + this.fullName() + "	delete
");
        })

        // Clear out the text boxes
        $("#input_first_name").val("");
        $("#input_last_name").val("");

    },

    // Remove customer from array and hide the row that contained them
    removeCustomer: function(e) {
        $(e.target).closest('tr').hide();
        customers.splice(e.target, 1);

    }
}

// Default customer object has to be created in order for unobtrusive calls to work
var cust = new Customer("el");

// unobtrusive jQuery event handling below
$("#btn_add_customer").click(function() {
    cust.addCustomer();
});

$("#btn_remove_cust").live("click", cust.removeCustomer);

$("#btn_log_customers_array").click(cust.logCustomers)

There are a few things that I'm concerned about:

I'm using jQuery to handle events and I'm trying to make everything as unobtrusive as possible.  In doing so, I wasn't exactly sure where I needed to place the event-handling code so I created a separate block below the prototype code to contain the button click events.  I'm not sure if this is the best way to go.
The events are being passed a function from the prototype - so in order for anything to work I have to new-up an instance of the 'Customer' before the user actually creates a new customer.  Seems like there might be a better way - but since I'm using a constructor function, I'm not sure if there is any other way around that.  If I change the constructor to an object literal, than I've just got a singleton.
Am I handling the removal of 'customers' from the array properly?  I created a simple utility function that is wired up to a button so that I could check and make sure that objects were being removed - as well as hidden in the DOM.

Here's are a few things that I noticed.

1

The control and flow of the application needs to be seperated into smaller logical groups.
The customers should be a container and controller for the customers, not the otherway around.
For example, Customer.prototype.addCustomer should be associated with Customers.prototype.addCustomer, since Customers is a container for customer objects.

2

this.id should be a property inside the Customer constructor. Ideally, the id value should be set to null until it is added to the Customers container.

3

Setting an object's prototype to an object over-writes the entire prototype, which means that you lose the reference to the constructor object.
So it's better if you add values and functions to the prototype one at a time like so.

// ok but you lost a few values.
Customer.prototype = {
    method1: function(){
        //...
    }
};

//best
Customer.prototype.method1 = function(){
    //...
};

4

Pass values to the constructor rather than have the constructor go out and find them.

// ok
var Customer = function(el) {
    this.el = document.getElementById(el);
    this.first_name = $("#input_first_name").val();
    this.last_name = $("#input_last_name").val();
    this.name = ""
}

// best
var Customer = function(el, firstName, lastName ) {
    this.el = document.getElementById(el);
    this.first_name = firstName;
    this.last_name = lastName;
    this.name = "";
       this.id;
}

Here's a free online book to help you to understand MVC
http://addyosmani.github.com/backbone-fundamentals/


Draw multiple objects The OpenGL tutorial that I was following only used a single model to demonstrate various points. I wanted to draw more than one model. After some research, I came to the conclusion that I needed to create a vertex array object, once for each model, and than I can use it to access buffers and attributes for each model. In order to do that, I am using this game object struct.

(I'm not including the whole code because it is quite long.)

typedef struct GameObject {
    GLuint VertexArray;
    GLuint NumVertices;
    kmMat4 ModelMatrix;
} GameObject;

For each object I want to draw, I am using loader function, such as the following:

#include "GameObject.h"

static const GLfloat triangle_vertices[] = {
    0, 1, 0,
    -1, -1, 0,
    1, -1, 0
};

static const GLfloat triangle_colors[] = {
    0.2, 0.2, 0.2,
    0.5, 0.5, 0.5,
    0.8, 0.8, 0.8
};

GameObject *newTriangle()
{
    GameObject *triangle = malloc(sizeof(GameObject));

    GLuint VertexBuffer;
    GLuint ColorBuffer;

    // 1. Generate VertexArray for object
    glGenVertexArrays(1, &(triangle->VertexArray));

    // Bind it
    glBindVertexArray(triangle->VertexArray);

    // Create and fill buffers vertex and color buffers
    glGenBuffers(1, &VertexBuffer);
    glBindBuffer(GL_ARRAY_BUFFER, VertexBuffer);
    glBufferData(GL_ARRAY_BUFFER, sizeof(triangle_vertices), triangle_vertices, GL_STATIC_DRAW);

    glEnableVertexAttribArray(0);
    glVertexAttribPointer(
           0,
           3,
           GL_FLOAT,
           GL_FALSE,
           0,
           (void*)0
    );

    glGenBuffers(1, &ColorBuffer);
    glBindBuffer(GL_ARRAY_BUFFER, ColorBuffer);
    glBufferData(GL_ARRAY_BUFFER, sizeof(triangle_colors), triangle_colors, GL_STATIC_DRAW);

    glEnableVertexAttribArray(1);
    glVertexAttribPointer(
           1,
           3,
           GL_FLOAT,
           GL_FALSE,
           0,
           (void*)0
    );

    // unbind vertex array
    glBindVertexArray(0);

    triangle->NumVertices = (sizeof(triangle_vertices)/sizeof(GLfloat));
    kmMat4Identity(&(triangle->ModelMatrix));

    return triangle;
}

And I am using this function to draw each game object:

#include "GameObject.h"

void Draw(GameObject *go, kmMat4 *VP, GLuint MatrixID)
{
    kmMat4 MVP;
    kmMat4Multiply(&MVP,VP,&go->ModelMatrix);

    // Bind vertex array
    glBindVertexArray(go->VertexArray);

    // set mvp matrix
    glUniformMatrix4fv(MatrixID, 1, GL_FALSE, MVP.mat);

    // Draw object
    glDrawArrays(GL_TRIANGLES, 0, go->NumVertices);

    // Unbind VertexArray
    glBindVertexArray(0);
}

And this is my main loop:

while (!glfwWindowShouldClose(window))
{
    kmMat4 current_translation;
    GLfloat translatex = 0;
    GLfloat translatez = 0;

    double current_time = glfwGetTime();
    double delta_time = current_time - last_time;
    last_time = current_time;

    if (glfwGetKey( window, GLFW_KEY_RIGHT ) == GLFW_PRESS){
        translatex += 2 * delta_time;
    }

    if (glfwGetKey( window, GLFW_KEY_LEFT ) == GLFW_PRESS){
        translatex -= 2 * delta_time;
    }

    if (glfwGetKey( window, GLFW_KEY_UP ) == GLFW_PRESS){
        translatez -= 2 * delta_time;
    }

    if (glfwGetKey( window, GLFW_KEY_DOWN ) == GLFW_PRESS){
        translatez += 2 * delta_time;
    }

    kmMat4Translation(&current_translation, translatex, 0, translatez);
    kmMat4Multiply(&square->ModelMatrix, &current_translation, &square->ModelMatrix);

    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);

    Draw(triangle, &vp, MatrixID);
    Draw(square,   &vp, MatrixID);

    glfwSwapBuffers(window);
    glfwPollEvents();
}

You are leaking the Vertex Buffer handles!

In your create function you allocated two Vertex Buffer Objects (VBOs), one for colors and one for positions, but you never store them anywhere, so your code will leak that memory, since those handles are no longer accessible after newTriangle returns. You should augment your GameObject structure to save those buffer handles and free them at a later time (with glDeleteBuffers). Since there's an allocation function, there should also be some sort of cleanup function in your code (e.g.: some freeGameObject or similar).

One VBO for just one triangle?

This is very inefficient. You should try to group stuff into the same VBO. Normally, you would put an entire 3D model into a buffer. You can very easily write a generic helper function that wraps the OpenGL calls for you. I'd also suggest declaring a helper structure that defines an interleaved vertex (more on this is a moment), so you can make efficient use of graphics memory by packing stuff into as little buffers as possible. Example:

typedef struct
{
    float px, py, pz; // vertex position
    float nx, ny, nz; // vertex normal for lighting
    float r, g, b;    // RGB vertex color
    float u, v;       // texture coordinates
    // anything else you may need
} my_gl_vertex;

// Then a helper to allocate the GL buffers for you,
// taking your generic vertex type:
void create_gl_buffers(const my_gl_vertex * verts_in, int vert_count,
                       GLuint * vao_out, GLuint * vbo_out);

So, what was that about interleaved vertexes again?

Just a fancy name to say that we are packing all the attributes of a model vertex into a structure, thus, we only allocate one VBO and place everything in it, rather than allocating one VBO for positions, one for colors, one for normals, etc. You can find examples of how to set this up in here. You should care about using interleaved data because it performs much better in the rendering, since all relevant data for a vertex will be tightly packed in memory, rather than scattered across different buffers. This will also allocate a single buffer, which might also reduce allocation overhead in the graphics driver side.

Move to indexed drawing

Last basic improvement you can add is to support indexed drawing, or indexed vertex specification. If you try, for instance, to draw a rectangle using two triangles, you'll see that you have to repeat two vertexes. This repetition can add up for larger and more complex 3D models, so if you instead supply a set of unique vertexes and use a separate set of integer indexes into this set of verts to describe the model, you get rid of all vertex duplication! The indexes will repeat, but they consist of small integer numbers, which should be a lot smaller than the average 3D vertex with color, position, whatnot. That's indexed drawing in a nutshell. You can check this tutorial for more details, or just look around in the web and you should find several more.


Getters and Setters for localStorage I have written the following get/set for a username which is stored in local Storage.  Do you think this "buys" me anything or even perhaps has major disadvantages?  I have never liked relying on global variables so I figured the getter and setter would be tidier and more maintainable.

var namespace = {};

namespace.username = {
        key: "Username",
        get: function() { return localStorage.getItem(this.key)},
        set: function(value) {localStorage.setItem(this.key, value)}
    };

before the code had "constants" (I know they arent real in JS)

LOCAL_STORAGE = {
    USERNAME: "Username"
}

and everywhere I need access to the username I would write:

localStorage.getItem(LOCAL_STORAGE.USERNAME);

I think this is a matter of taste, I don't see any major disadvantages but no major advantages either. In these cases I would err on the side of simplicity and go with querying localStorage directly.

Now to fully whet my morning CR.SE appetite let's get really nitpicky:

Pros

It will make it marginally easier to change your code if you some day decide to use a different storage mechanism (for example a fallback mechanism if localStorage doesn't exist). This is only marginally easier unless your code is partitioned in such a way that a global search for LOCAL_STORAGE.USERNAME won't work
From the standpoint of coding practice, it IS generally considered better to hide all your external dependencies, and a good argument can be made that localStorage is one
Gets rid of your LOCAL_STORAGE keys object
Getters and setters are familiar to people coming from a java background

Cons

You actually have more global objects now, as window.namespace.username and all it's children are now effectively global (though namespaced)
If someone picks up your code they might not see the localStorage references and assume that you don't depend on it. They might therefore make decisions (for example to deploy to an old IE) that might damage stability. (this is the counterpoint to abstracting away external dependencies).
Getters and setters are icky and anti-intuitive for someone not coming from the java world
Someone has to learn your api to use it the way you want it to be used. While your code makes the assumption that they are accessing username through your object, someone could very reasonably pick up the code, open up devtools, see that there is a Username key in localStorage, and start querying it directly thereby breaking your expectation. In short, it's so easy to access localstorage, there's no reason for them to expect that you abstracted it.
Are you going to follow the same pattern for other objects too? You really should pick one pattern and stick to it and only make exceptions when there is a good reason to.
Because you're using this someone can use new, or call, or apply and break your username functions

General Notes

localStorage.getItem('Username') is in no way worse than localStorage.getItem(LOCAL_STORAGE.USERNAME). I would wager it's better because it is less typing and more obvious. Butbutbut it's a magic string! This is javascript, not java, it's all magic strings. Note that my own personal preference is to use ' to differentiate a token (similar to ruby's :token) from text for which I use ".
LOCAL_STORAGE - I really dislike that naming convention. Whatever works for you I guess, but what is the semantic meaning of all-caps? How is it semantically different than namespace.localStorageKeys?
Do you really need a setter? I image that you're only going to be setting Username in one place - why make a global method then?
So you still want to abstract away localStorage? How about a more javascript-y approach?

myNamespace.storage = function(key, val) {
  return val === undefined ?
  localStorage.getItem(key) : localStorage.setItem(key, val);
}

then:

myNamespace.storage('Username', "George");
myNamespace.storage('Username') == "George";

This is of course a very naive implementation, you could probably add some more logic handling serializing/deserializing objects from json and fallback mechanisms.


Jinja2 block that is unreadable I have a block of Jinja2 (used in Sceptre/CloudFormation). It is not readable in my opinion, but I can't find any way to clean it up to improve it.

{% set args = "--kubelet-extra-args '--node-labels=nodegroup=" + sceptre_user_data.node_group_name %}

{% if sceptre_user_data.node_labels != "None" %}
  {% set args = args + "," + sceptre_user_data.node_labels %}
{% endif %}

zone=$(curl -s http://169.254.169.254/latest/meta-data/placement/availability-zone)

{% if sceptre_user_data.cni_custom_network == "Yes" %}
  {% set args = args + ",k8s.amazonaws.com/eniConfig=pod-netconfig-$zone" %}
{% endif %}

{% if sceptre_user_data.taints != "None" %}
  {% set args = args + " --register-with-taints=" + sceptre_user_data.taints %}
{% endif %}

{% set args = args + "'" %}

eval "/etc/eks/bootstrap.sh ${EKSClusterName} {{ args }}"

I tried using join filters although that seemed to make it worse.

(This has nothing to do with Flask, but is all about Jinja2 obviously. But there appears to be no Jinja2 tag.)

In the end I refactored like this:

{% set node_group_name = sceptre_user_data.node_group_name %}
{% set node_labels = sceptre_user_data.node_labels %}
{% set taints   = sceptre_user_data.taints %}
{% set cni_custom_network = sceptre_user_data.cni_custom_network %}

...

do_bootstrap() {
  {% set args = "--kubelet-extra-args '--node-labels=nodegroup=" + node_group_name %}

  {% if node_labels != "None" %}
    {% set args = args + "," + node_labels %}
  {% endif %}

  {% if cni_custom_network == "Yes" %}
    zone=$(curl -s http://169.254.169.254/latest/meta-data/placement/availability-zone)
    {% set args = args + ",k8s.amazonaws.com/eniConfig=pod-netconfig-$zone" %}
  {% endif %}

  {% if taints != "None" %}
    {% set args = args + " --register-with-taints=" + taints %}
  {% endif %}

  {% set args = args + "'" %}

  eval "/etc/eks/bootstrap.sh ${EKSClusterName} {{ args }}"
}

do_bootstrap

I would still welcome feedback from anyone who knows a Jinja2 trick that might help that I don't know.


Handler for incoming network packages I want to let you look through my NetHandler which is basically the component which gets notified when a full packet arrived (not like tcp-packet but my own type of packets) and handles it.

First of all, the handle method which gets called from the networking

void NetHandler::handle(int id, char* data, int len) {
#define P(ID, TYPE) case ID: try_handle<TYPE>(data, len); break
    switch (id) {
        P(0, game::networking::packets::Hello);
        P(1, game::networking::packets::Disconnect);
        P(2, game::networking::packets::Chat);
    default:
        unhandled(id);
    }
#undef P
}

Yeah... As you can see, a little define to create a case for me. unhandled just prints out an error.

Next, the try_handle<>()

template<typename T>
void try_handle(char* data, int len) {
    T packet;
    if (!packet.ParseFromArray(data, len)) {
        bad_packet(packet.GetTypeName(), len);
        return;
    }
    handle(packet);
}

It creates the actual corresponding Packet and parses it (I use google protobuf). bad_packet, again, just prints an error. Now the handle

#define STUB(TYPE) virtual void handle(TYPE& type) { unhandled(TYPE::default_instance().GetTypeName()); }
    STUB(game::networking::packets::Hello)
    STUB(game::networking::packets::Disconnect)
    STUB(game::networking::packets::Chat)
#undef STUB

I create a virtual method for every packet-type which calls unhandled. In my application I can now subclass this NetHandler and override the handler-methods for all packets I expect.

Now the other direction, send packets. I actually wanted a single send-method which takes every possible packet and just sends it. But it needs to send the id along with it, so I created a PacketRegistry

class PacketRegistry {
public:
    PacketRegistry() {
        for (int i = 0; i < 256; i++)
            messages[i] = nullptr;
        reg<game::networking::packets::Hello>(0);
        reg<game::networking::packets::Disconnect>(1);
        reg<game::networking::packets::Chat>(2);
    }

    template<typename T>
    void reg(int id) {
        messages[id] = T::default_instance().GetDescriptor();
    }

    int getId(const google::protobuf::Message& message) {
        const google::protobuf::Descriptor* ptr = message.GetDescriptor();
        for (int i = 0; i < 256; i++) {
            if (messages[i] == ptr)
                return i;
        }
        return -1;
    }

    const google::protobuf::Descriptor* messages[256];
};

As you can see, this is the third time I need to add every packet manually.

I tried to come up with some template-magic, but I'm not that experienced yet.
Something like

//in NetHandler
template<typename T>
void handle(T& packet) { unhandled(T::default_instance().GetTypeName()); }

//In a subclass
template<>
void handle<game::networking::packets::Chat>(game::networking::packets::Chat& chat) {   }

unfortunately doesn't work because... well... templates and virtual don't really like each other.

Any ideas how I can let the template-magic do more of the dirty work instead of defining methods, cases, and so on for every existing packet?

Few suggestions:

Add/use inheritance for packet type (assuming you have something like game::networking::packets::PacketTypeBase that all packet types inherit from. Have GetDescriptor()/GetTypeName()/handle() on this base class
Create a factory to go from int id to packet Type. Something like: game::networking::packets::PacketTypeBase make_packet(int id);
Add a virtual function handleInput(char* data, int len) so your NetHandler::handle simply becomes return make_packet(id).handleInput(data,len). The default can be handled by base class or separate UnknownPacket child class.

I would recommend using inheritance on packet type, not NetHandler. If you need flexibility, you can even pass a functor into the ctor of NetHandler if you want different work done on the PacketType by each instance of NetHandler

In general try to replace switch statement with inheritance and virtual functions. Factory/maker function comes in handy even during testing. Common functionality can be put in base class so you can avoid duplicate code (not need macros) & still do packet specific work.

Instead of having a hard-coded list of 256 (avoid magic numbers), use a std::map / std::unordered_map/HashMap so find / lookup will be faster in getId (use a data structure that  optimizes your most common case)

Hope this helps.