Brainf**k to Ruby converter, written in Ruby -- v1.0 I've seen a lot of Brainf**k interpreters in various languages. I decided that actually interpreting Brainf**k is too hard, so instead of that, I wrote a 'compiler' in Ruby that directly transcribes BF to Ruby code. This is Version 1.0, and I'm looking for any tips on how to make it more efficient.

There are a few things that really bug me:

replacements feels like it's messed up somehow, but I can't put my finger on how.
$stdin.readbyte feels entirely too long, but I can't find an alternative that's shorter.
Should I be using $stdin at all?

input_file = $ARGV[0]
output_file = $ARGV[1]
start = <<-END.gsub(/[ \t]*\|/, '')
  |# Automatically generated by bf_to_ruby.rb
  |# Source available at GitHub
  |data = Hash.new(0)
  |pointer = 0
END

replacements = [
    { replacing: '+', replacement: 'data[pointer] += 1'              },
    { replacing: '-', replacement: 'data[pointer] -= 1'              },
    { replacing: '>', replacement: 'pointer += 1'                    },
    { replacing: '<', replacement: 'pointer -= 1'                    },
    { replacing: '.', replacement: 'putc data[pointer]'              },
    { replacing: ',', replacement: 'data[pointer] = $stdin.readbyte' },
    { replacing: '[', replacement: 'until data[pointer] == 0'        },
    { replacing: ']', replacement: 'end'                             },
]

output = open(output_file, File::CREAT | File::WRONLY)
output.puts(start)

open(input_file, File::RDONLY) do |input|
  input.each_char do |char|
    replacements.each do |data|
      if data[:replacing] == char
        output.puts(data[:replacement])
      end
    end
  end
end

Demo

Input:

++++++++[>++++[>++>+++>+++>+<<<<-]>+>+>->>+[<]<-]>>.>---.+++++++..+++.>>.<-.<.+++.------.--------.>>+.>++.

Output:

# Automatically generated by bf_to_ruby.rb
# Source available at GitHub
data = Hash.new(0)
pointer = 0
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
until data[pointer] == 0
pointer += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
until data[pointer] == 0
pointer += 1
data[pointer] += 1
data[pointer] += 1
pointer += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
pointer += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
pointer += 1
data[pointer] += 1
pointer -= 1
pointer -= 1
pointer -= 1
pointer -= 1
data[pointer] -= 1
end
pointer += 1
data[pointer] += 1
pointer += 1
data[pointer] += 1
pointer += 1
data[pointer] -= 1
pointer += 1
pointer += 1
data[pointer] += 1
until data[pointer] == 0
pointer -= 1
end
pointer -= 1
data[pointer] -= 1
end
pointer += 1
pointer += 1
$stdout.putc data[pointer]
pointer += 1
data[pointer] -= 1
data[pointer] -= 1
data[pointer] -= 1
$stdout.putc data[pointer]
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
$stdout.putc data[pointer]
$stdout.putc data[pointer]
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
$stdout.putc data[pointer]
pointer += 1
pointer += 1
$stdout.putc data[pointer]
pointer -= 1
data[pointer] -= 1
$stdout.putc data[pointer]
pointer -= 1
$stdout.putc data[pointer]
data[pointer] += 1
data[pointer] += 1
data[pointer] += 1
$stdout.putc data[pointer]
data[pointer] -= 1
data[pointer] -= 1
data[pointer] -= 1
data[pointer] -= 1
data[pointer] -= 1
data[pointer] -= 1
$stdout.putc data[pointer]
data[pointer] -= 1
data[pointer] -= 1
data[pointer] -= 1
data[pointer] -= 1
data[pointer] -= 1
data[pointer] -= 1
data[pointer] -= 1
data[pointer] -= 1
$stdout.putc data[pointer]
pointer += 1
pointer += 1
data[pointer] += 1
$stdout.putc data[pointer]
pointer += 1
data[pointer] += 1
data[pointer] += 1
$stdout.putc data[pointer]

Both, when run, output:

Hello World!

You should open the input file before the output file, so that if the input file is unreadable, no file is created at all.  File opening should be done using blocks for automatic closure and cleanup.

This is too complicated:

replacements.each do |data|
  if data[:replacing] == char
    output.puts(data[:replacement])
  end
end

replacements would be better as a Hash:

replacements = {
    '+' => 'data[pointer] += 1',
    '-' => 'data[pointer] -= 1',
    '>' => 'pointer += 1',
    '<' => 'pointer -= 1',
    '.' => 'putc data[pointer]',
    ',' => 'data[pointer] = $stdin.readbyte',
    '[' => 'until data[pointer] == 0',
    ']' => 'end',
}

open(input_file, File::RDONLY) do |input|
  open(output_file, File::CREAT | File::WRONLY) do |output|
    output.puts(start)
    output.puts(input.each_char.map { |c| replacements[c] }.compact.join("\n"))
  end
end

One big puts should be faster than many indivual puts.


Checking host status for clients on server I run a small server, and to check whether clients are being hosted on my server, I wrote this.  I'm new to object oriented programming. How stratified should my programs really be?  Do I want to have every possible variable and piece in their own function? Also, how am I doing in general?  Any recommendations?

class Domain_check
  def vhost_grab
    return full_domain_path = `ls /etc/httpd/conf.d/vhost_* | grep -v 000_defaults.conf`.chomp.split(' ')
  end

  def vhost_stripper
    prefix =  Domain_check.new.vhost_grab
    vhost_stripped = []
    prefix.each_index do |x|
      vhost_stripped[x] = `echo '#{prefix[x]}' | awk -F'vhost_' '{print $2}' | awk -F'.conf' '{print $1}'`
    end
    return vhost_stripped
  end

  def vhost_display
    puts "\n%s %40s %43s" %["Domain name", "IP Address Listed", "IP Address Currently in Use"]
    final_vhost = vhost_stripper
    final_vhost.each_index { |x|
      padding = 50
      print final_vhost[x].strip
      padding = padding.to_i - final_vhost[x].length
      print "%0#{padding}s" %[`grep '<VirtualHost .*:80>' #{Domain_check.new.vhost_grab[x]} | awk -F'<VirtualHost' '{print $2}'|awk -F':' '{print $1}'`.strip.to_s]
      puts "%40s" %[`dig #{final_vhost[x].strip} +short`]
    }
  end
end

d1 = Domain_check.new
d1.vhost_display

Some observations:

Regarding the "stratification". It's hard to give practical advice, this is something that comes from experience and personal taste. Some basics to start with: a) write classes/modules with low coupling to achieve real modularization, b) write fairly short methods than do only one thing. c) Keep model-view separations. d) Don't use global variables.
class Domain_check: that's against Ruby practices, a class or module names are named CamelCase: class DomainCheck.
Don't write an explicit return on the last line of a method/block, it's non idiomatic.
Don't assign a variable on this last expression, why would be it different from the method name itself?
Don't call to external commands (ls, grep, awk, ...), Ruby is more than able to perform those tasks, check the standard library.
About this: prefix =  DomainCheck.new.vhost_grab. You are already in the instance, just prefix = vhost_grab.
You are using a class just nominaly, you don't use any of its facilities. You can convert all these methods to classmethods if you store nothing in the instances.
Init empty + iterate with each + push is an anti-pattern in Ruby (and in any language with decent functional capabilities, for that matter). Use Enumerable#map. Also, you use each_index to iterate in a C fashion, use each (or better, map, select, inject, as required, read the Enumerable documentation from start to finish).
Having only a C background your code has a serious problem, it's very, very imperative. Functional programming allows far better abstraction and  clarity, check this wiki page I maintain.
If every method is prefixed with vhost_ there's no point in prefixing anything. Besides, if all the methods start with vhost_, the class should probably be called VirtualHostChecker instead (@Flambino).


Inverting 3rd party dependency Looking for a way to efficiently use 3rd party CSV reader without making a dependency on it. I would also prefer to make my class be available everywhere (not just in IoC container injectable Services), so I decided to go with a questionable decision – configure through the static property. Can you see a way to reduce amount of library code or make configuration more obvious without losing library handiness?

Repository at GitHub.

Project dependencies are:

And solution looks like this:

MyProject.Demo registers CSV library parser dependency and parses the CSV file:

class Program
{
    static void Main(string[] args)
    {
        CsvRowReader.Use();

        var text = "First,Last\nJohn,Doe\n";
        var file = NameFile.Parse(text);
        foreach (var name in file)
            WriteLine($"{name.First} {name.Last}");
    }
}

Business logic assembly MyProject defines NameFile as:

public class NameFile : RowFile<NameFile, FullName>
{
    protected override IEnumerable<FullName> Read(RowReader reader)
    {
        using(reader)
            while (reader.Read())
                yield return new FullName
                {
                    First = reader.Get<string>("First"),
                    Last = reader.Get<string>("Last")
                };
    }
}

Where FullName is:

public class FullName
{
    public string First { get; set; }
    public string Last { get; set; }
}

Using library class RowFile<TFile, TRow>:

public abstract class RowFile<TFile, TRow> : Enumerable<TRow>
    where TFile : RowFile<TFile, TRow>, new()
{
    public static readonly TFile Empty = new TFile();

    public static TFile Parse(string text) =>
        Load(new StringReader(text));

    public static TFile Load(string filePath) =>
        Load(File.OpenText(filePath));

    public static TFile Load(Stream stream) =>
        Load(new StreamReader(stream));

    public static TFile Load(TextReader reader)
    {
        var file = new TFile();
        file.Rows = file.Read(RowReader.Create(reader));
        return file;
    }

    public sealed override IEnumerator<TRow> GetEnumerator() =>
        Rows.GetEnumerator();

    protected abstract IEnumerable<TRow> Read(RowReader read);
    IEnumerable<TRow> Rows { get; set; } = new TRow[0];
}

Where:

public abstract class RowReader : IDisposable
{
    public static Func<TextReader, RowReader> Create { get; protected set; }
    public abstract void Dispose();
    public abstract bool Read();
    public abstract T Get<T>(string name);
}

And:

public abstract class Enumerable<T> : IEnumerable<T>
{
    public abstract IEnumerator<T> GetEnumerator();
    IEnumerator IEnumerable.GetEnumerator() =>
        GetEnumerator();
}

3rd party CSV reader dependency is incapsulated in MyProject.CsvHelper project:

public class CsvRowReader : RowReader
{
    public static void Use() =>
        Create = reader => new CsvRowReader(reader);

    CsvRowReader(TextReader reader)
    {
        Reader = new CsvReader(reader);
        Reader.Read();
        Reader.ReadHeader();
    }

    CsvReader Reader { get; }

    public override void Dispose() =>
        Reader.Dispose();

    public override bool Read() =>
        Reader.Read();

    public override T Get<T>(string name) =>
        Reader.GetField<T>(name);
}

I don’t like all those mutability things, but do not see a better solution at the moment...

public static TFile Load(TextReader reader)
{
    var file = new TFile();
    file.Rows = file.Read(RowReader.Create(reader));
    return file;
}

In order to get rid of the mutable property file.Rows I suggest taking a look at Autofac's factory delegate for creating TFile immutably and injecting this delegate as a dependency.

Alternatively you could register them via named and keyed services and pick the right factory by TKey with Type as key. I use this techniques quite often.

I guess other dependency injection frameworks could do similar magic.


Water accumulated between towers I came across this interview question to find water accumulated between towers:

  You are given an input array whose each element represents the height of the tower. The width of every tower is 1. You pour water from the top. How much water is collected between the towers?

Any suggestions?

def test(x):
    if len(x) <= 2 or x == sorted(x) or x == sorted(x, reverse=True):
        return 0

    area = 0
    count = 1
    max_height = x[0]

    while count < len(x) - 1:
        if x[count] < max_height and x[count:] != sorted(x[count:], reverse=True):
            temp_count = 0
            new_count = count
            temp_height = 0
            height = max_height
            while count < len(x):
                if x[count] >= height or count == len(x) - 1:
                    temp_count = count
                    temp_height = x[count]
                    height = min(max_height, temp_height)
                    break
                count += 1
            while new_count < temp_count:
                area += height - x[new_count]
                new_count += 1
        else:
            max_height = x[count]
            count += 1

    return area

try:
    a = [1,2,3,4,5]
    print a, test(a)

    b = [5,4,3,9,1]
    print b, test(b)

    c = [3,2,1,1,1,2,3]
    print c, test(c)

    e = [1,2,0,2,1]
    print e, test(e)

    f = [1, 4, 2, 5, 1, 2, 3]
    print f, test(f)

    g = [3, 2, 1, 1, 1, 2, 3]
    print g, test(g)

    a1 = [1001,1000,1002]
    print a1, test(a1)
except Exception as e:
    print e

Three quick comments:

There’s no documentation anywhere in your code. Ideally there should be a docstring, and some comments explaining why the code is behaving in a particular way. I wasn’t able to work out your approach to the problem because there are no comments, and I didn’t want to sit down and reverse engineer it.

Note that variable names are also a form of documentation, and some of yours could be better – x is a poor name, and so is count. What’s it counting?
Your tests aren’t very useful, because I have to work out if they're correct by hand. If I start editing the code, and I introduce a bug, there’s a possibility I won’t notice the test changes. Your tests should warn me if there’s been a regression, not simply print out some results.
The most basic form of this is something like.

assert test(a) == 0
assert test(b) == 3
assert test(c) == 8

Now I can read your tests and find out what the results should be, and I can be warned if I introduce a bug.

The better way to do tests is with the unittest module. If you haven’t used that before, I recommend Ned Batchelder’s talk Getting started with testing, which is a good introduction to Python testing in general.
I don’t know why you have the try…except block. You should really only use try…except if you expect an exception to be raised, so that you can wrap it accordingly. Unexpected exceptions tell you something about your program – there’s a problem – and you shouldn’t be ignoring that information.

And you should try to avoid broad except: statements. Better to catch only the specific exceptions you think might be raised (in this case, there aren’t any), and let anything else bubble to the top.


Modular Exponentiation I want to solve the following for \$x\$ (in Python):

\$x^{101} = 8765 \ (\mod{9691573})\$

I coded this:

n = 8765
power = 101
mod = 9691573

x = 0
while x < mod:
    if (x ** power) % mod == n:
        break
    x += 1

print x

This is basically the "brute force" approach.

What is the most efficient way to solve this problem programmatically?

Note:

  solving the equation \$x^n = a \mod{M}\$ for the unknown \$x\$ in modular arithmetic means to find an integer \$x\$ such that \$x^n - a\$ is a multiple of the modulus \$M\$

Note: this is not exactly the RSA problem. For it to be the RSA problem, the modulus would have to be composite. Turns out, in this case, the modulus is prime.

Note also: this is not the discrete log problem, as in the discrete log problem we are trying to find the exponent, not the base.

That said, you can find the answer using the method that CodesInChaos recommended.

Factor mod, easy, mod is already prime, so it is factored.
Compute phi(mod). This is the euler totient function. For primes, phi(mod)=mod-1. So in your case, phi(mod)=9691572.
Compute d such that power*d=1 modulo phi(mod). Use the extended euclidean algorithm to do this. You will get d=7868405. There are python libraries that will do it for you (pycrypto Crypto.Util.number.inverse)
Compute 8765^d modulo mod. In python you want to do this using pow. pow takes a third argument (a modulus) which is way optimized when compared to ** followed by %. It does the square-and-multiply method. In this case, pow(8765, 7868405, 9691573) returns 680457.

You can check the answer by doing pow(680457, 101, 9691573) and make sure it return 8765.

If the modulus were not prime, you'd have to factor it to compute phi(mod). After that, everything else is the same.


The Telegram bot for book information Telegram Bot that retrieves book information from GoodReads

I've created a Telegram bot that implements the inline/offline modes to get the brief info about a book, searched by title(inline mode) or with command /random. The data is retrieved from GoodReads. I was wondering how can I modify the body of 'description', so that I would be able to add here that rating info and name of the author? The "idea" of the interpretation is as an example of IMDB's bot

The GoodReads client

import goodreads
from goodreads import client
import random

client_key = '[redacted']
client_secret = '[redacted']

gc = client.GoodreadsClient(client_key, client_secret)

def parse_book(book):
    return {
        'title': book_data.title,
        'author': book_data.authors[0],
        'rating': book_data.rating,
        'review': book_data.review,
        'cover': book_data.image_url,
        'link': book_data.link
    }

  def book_info(title):
      books = gc.search_books(str(title), page=1, search_field='all')

      return map(parse_book, books)

  def random_book():
      index = random.randint(1, 1000000)
      book = gc.book(index)

      return parse_book(book)

The Telegram Bot implementation

import telebot
from telebot import types
import gd

token = '[redacted']
bot = telebot.TeleBot(token)

@bot.message_handler(commands=['start'])  # greeting
def send_welcome(message):
    bot.reply_to(message.chat.id, "Hi! How are you?")

@bot.message_handler(commands=['help'])  # command list
def send_welcome(message):
    bot.reply_to(message.chat.id, "This bot can do: ")

@bot.message_handler(commands=['random']) # /random
def random(message):
    book = random_book()
    text = "Some text"
    bot.send_message(message.chat.id, text, disable_web_page_preview=False)

@bot.inline_handler(lambda query: query.query == 'text')  # inline session
def query_text(query):
    raise Http404("Poll does not exist")
    book_req = book_info(query)
    try:
        result = []
        i = 0
        for book in book_req:
            result.append(types.InlineQueryResultArticle(id=i,
                title=book[i]['title'],
                url=book[i]['link'],
                hide_url=True,
                description=book[i]['review'],
                thumb_url=book[i]['cover'], thumb_width=48, thumb_height=48
                ))
            i += 1
        bot.answer_inline_query(query.id, result)
    except Exception as e:
        print("{!s}\n{!s}".format(type(e), str(e)))

def main_loop():
    bot.polling(True)
    while 1:
        time.sleep(3)

if __name__ == '__main__':
    try:
        main_loop()
    except KeyboardInterrupt:
        print >> sys.stderr, '\nExiting by user request.\n'
        sys.exit(0)

gc is the name of the Garbage Collection interface module. It shouldn't matter too much, but, when reading your code, it feels like that you are asking the garbage collector to find books and then I remind myself that it isn't actually the garbage collector.

index = random.randint(1, 1000000)

This was mentioned in the last post. Even if you can't find the specific number you need, you should at least assign 1000000 to some variable something like MAX_BOOK_NO so the meaning of such a number is more obvious.

    i = 0
    for book in book_req:
    ...
        i += 1

This is simplified with enumerate:

    for i, book in enumerate(book_req):
    ...


If zero found in Matrix. make entire row & column zero Problem is - If in an MxN matrix, a zero is encountered, the entire row and entire column should be set to zero.

Example -

8   4   2
2   3   7
1   0   1
9   8   4

Should become:

8   0   2
2   0   7
0   0   0
9   0   4

I encountered this problem today and came up with a solution.
I'm hoping for some feedback regarding the time/space complexity and how efficient my solution is :

Code -

 public static void zeroMatrix(int[][] arr1)
{
    ArrayList<Integer> coord = new ArrayList<>();

    int row = arr1.length;
    int column = arr1[0].length;
    for(int i=0; i < row; i++)
    {
        for(int j=0; j < column; j++)
        {
            if(arr1[i][j]==0)
            {
                coord.add((10*i) + j);
            }
        }
    }

    for(int n : coord)
    {
         int j=n%10;
         int i=n/10; int k=0;
        int l=0;
        while(k<row)
        {
            arr1[k][j]=0;
            k++;
        }
        while(l<column)
        {
            arr1[i][l]=0;
            l++;
        }
    }
}

What I'm doing is:

Saving the coordinates of found zeroes in an ArrayList as a two
digit integer.
Taking out ones and tens places from the stored
list (which are the coordinates) and running a loop over the found
zeroes only.
Then making the corresponding rows and columns 0

Implementation

You did it right that you first find the zero entries and only after that set the rows/columns to zero. However, you can come up with more cohesive and flexible code:

public static void zeroMatrix(final int[][] matrix) {
    findZeroEntries(matrix).forEach((p) -> {
        setToZero(matrix, p.x, p.y);
    });
}

private static List<Point> findZeroEntries(final int[][] matrix) {
    final int height = matrix.length;
    final int width  = matrix[0].length;
    final List<Point> zeroEntryList = new ArrayList<>();

    for (int y = 0; y < height; ++y) {
        for (int x = 0; x < width; ++x) {
            if (matrix[y][x] == 0) {
                zeroEntryList.add(new Point(x, y));
            }
        }
    }

    return zeroEntryList;
}

private static void setToZero(final int[][] matrix,
                              final int x,
                              final int y) {
    setRowToZero(matrix, y);
    setColumnToZero(matrix, x);
}

private static void setRowToZero(final int[][] matrix, final int y) {
    final int width = matrix[0].length;

    for (int x = 0; x < width; ++x) {
        matrix[y][x] = 0;
    }
}

private static void setColumnToZero(final int[][] matrix, final int x) {
    final int height = matrix.length;

    for (int y = 0; y < height; ++y) {
        matrix[y][x] = 0;
    }
}

Coding conventions

Braces

What comes to braces, the recommended way to write them is to put the opening brace ({) on the same row with the statement that belongs to it. Instead of

for (...)
{
    ...
}

you should write

for (...) {
    ...
}

Spaces

For the sake of readability, you should have a single space character after each for and if, and a single space character before and after any binary operator. So instead of

for(int i=0; ...) {
    if(i==1) {

    }
}

you should have

for (int i = 0; ...) {
    if (i == 1) {
        ...
    }
}

Program to interface

Not that it makes much difference in your method, but usually the rule goes that people should program to an interface, and not to an implementation. So instead of

ArrayList<Integer> coord = new ArrayList<>();

you should write

List<Integer> coord = new ArrayList<>();

Naming

arr1 is not the best possible name. Why not use, say, matrix?

Also

int row = arr1.length;
int column = arr1[0].length;

above I would have used the plural form:

int rows = arr1.length;
int columns = arr1[0].length;

Zero entry coordinates

As @200_success mentioned, packing the coordinates the way you do is not a good idea.

Hope that helps.


Extension methods, is this too dirty? I love lambdas, and functional programming etc. But sometimes I wonder if I take it too far..

public static T With<T>(this T source, Action<T> action)
{
    action(source);
    return source;
}

public static IEnumerable<T> ForEach<T>(this IEnumerable<T> source, Action<T> action)
{
    return source.Select(x => x.With(action));
}

thoughts?

UPDATE

Some good points, some I agree with, some I don't, but a few details:

I understand that Linq may not be intended to handle side effects, especially since List.ForEach returns void, however I disagree that functional programming can't contain side effects, a monad is simply a chunk of code to execute for a given structure.

The ForEach and With extensions, are meant to be a syntactic sugar.  Perhaps the name is poorly chosen, and it should be something like OnEnumerate, however I wonder if this could get confusing down the line, as its whole point is to invoke side effects at that particular point in the expression, and this could occur multiple times, perhaps I should stick with my With convention and do WithEach.

The comments about using Select internally are probably true, as I'm not really projecting, and yield return saves a method call. (premature optimization anyone?)

public static IEnumerable<T> WithEach<T>(this IEnumerable<T> source, Action<T> action)
{
    foreach(var x in source)
        yield return x.With(action);
}

To be honest I probably wouldn't use WithEach often, unless its an edge case where I'm passing an IEnumerable<T> that may or may not be enumerated. I can imagine it being effective in generating complex hierarchies. However With I use quite often, usually as a way to chain things together, i.e:

this.SomeObservable = someObservable
    .With(o => o.Subscribe(this.OnNext)
        .With(this._disposables.Add));

My advice: don't do this.

Building devices that encourage you to write expressions that are useful only for their side effects is poor functional style.
You are abusing "Select" when you use it as an implementation detail of "ForEach". "Select" does not have the semantics of "ForEach"!  The purpose of "Select" is to compute a projection, not to cause a side effect.
You are building yourself a little time bomb. Because your implemenation of ForEach is to build a Select query, the "ForEach" executes lazily.  When you say "blah.ForEach(whatever);" that doesn't do anything. It just builds up an object that represents doing the side effects in the future.  Since you never use that object for anything, the side effects remain undone.  If, by contrast, you do use that object, you run the risk of using it twice and causing the same side effects again.

That said, the idea of what you're doing is sound; the idea of representing a sequence of side-effecting operations in a lazily-evaluated object is how side effects are represented in Haskell. But if that's what you want to do, then actually build yourself a real monad that clearly and unambiguosly represents the semantics of "I'm building an object that represents a sequence of side effects in the future". Don't be all coy about it! Be explicit that that's what you're doing.


AppTaskWorker - Running class methods at sporadic intervals I decided to write a class to do this for me. I had a lot of methods that needed to be invoked at sporadic intervals, across multiple projects.

The dictionary will eventually be class methods, I just put new Task()... as an example. I'm fairly new to asynchronous programming, so constructive criticism is 100% welcome.

I set a low interval (10s) on  the main timer because I want to stay as close to the interval in the dictionary as possible. I'm not sure the lowest I can put it without ruining performance.

At some point, I plan to implement non async methods. If anyone has any ideas on how I should implement that, please let me know. I feel like running a non async method inside a task would be a bad idea, maybe a seperate dictionary and process method would work?

Thanks.

public class AppTaskWorker : IAppTaskWorker
{
    private readonly ILogger _logger;
    private readonly ConcurrentDictionary<Task, DateTime> _tasksLog;
    private readonly ConcurrentDictionary<int, Task> _tasks;

    public AppTaskWorker()
    {
        _logger = new ConsoleLogger(typeof(AppTaskWorker));
        _tasksLog = new ConcurrentDictionary<Task, DateTime>();
        _tasks = new ConcurrentDictionary<int, Task>();

        TryRegisterTask(30, new Task(() => // every 30 seconds
        {
            Console.WriteLine("Task 1 has been processed.");
        }));

        TryRegisterTask(120, new Task(() => // every 2 minutes
        {
            Console.WriteLine("Task 2 has been processed.");
        }));

        TryRegisterTask(25, new SomeController().Process()); // every 25 seconds

        Start();
    }

    public void Start()
    {
        while (true)
        {
            Task.Run(async () =>
            {
                await ProcessActionsAsync();
                await Task.Delay(10000);
            });
        }
    }

    public bool TryRegisterTask(int interval, Task task)
    {
        return _tasks.TryAdd(interval, task);
    }

    private async Task ProcessActionsAsync()
    {
        foreach (var keyValuePair in _tasks)
        {
            var task = keyValuePair.Value;

            if (!_tasksLog.TryGetValue(task, out var lastProcessed))
            {
                continue;
            }

            if (!((DateTime.Now - lastProcessed).TotalSeconds >= keyValuePair.Key))
            {
                continue;
            }

            _tasksLog[task] = DateTime.Now;

            try
            {
                await task;
            }
            catch (Exception e)
            {
                _logger.Error(e.ToString());
            }
        }
    }
}

Start() is spawning an infinite number of new tasks without waiting for them to complete. It's also a bit weird that Start() is blocking the calling thread.

Make Start() async and await the Task returned by Task.Run.

Start(), a blocking method, being called at the end of the constructor is an absolute no-go. The constructor is not done, the object initialization is never completed.

Unless you're living on UTC (DateTime.Now - lastProcessed).TotalSeconds is going to be inconsistent around daylight saving transitions.


Returning status codes from business layer This is what I've been doing so far in my business layer to return error messages to the user.

public void CreateMerchant(MerchantCreationModel model, out MerchantCreationStatus status)
{
    //check if merchantId exists
    MerchantDTO merchant = _merchantRepo.GetMerchant(model.MerchantId);

    if (merchant != null)
    {
        status = MerchantCreationStatus.MerchantAlreadyExists;
        return;
    }

    // Rest of merchant creation logic has been removed for clarity

    status = MerchantCreationStatus.Success;
}

I have a ErrorCodeToString() method in my controller that gets the text to display.

What do you think of this technique?

Status codes have long gone the way of the Dodo --- made extinct by exceptions, actually.

The problem with a status code is that it can be ignored. An exception cannot. Consider these two examples.

Status Codes as Return Values

First, the "merchant repository" implementing CRUD operations for merchants:

public enum MerchantStatusCode
{
    OK = 0,
    Created = 1,
    Updated = 2,
    Deleted = 3,
    MerchantAlreadyExists = 4,
    MerchantDoesNotExist = 5
}

public class MerchantRepository
{

    public MerchantStatusCode Create(Merchant model)
    {

        if (MerchantExists(model))
            return MerchantStatusCode.MerchantAlreadyExists;

        // Save to DB

        return MerchantStatusCode.Created;
    }
}

Now, the C# code to create a merchant and ignore error handling:

var merchantRepository = new MerchantRepository();

// Create a merchant. I'm not really sure what could go wrong, and if
// something should go wrong, I'm not really sure if it's a problem.
merchantRepository.Create(merchant);

The MerchantRepository.Create method returns a status code that our programmer promptly ignores. By looking at these lines of code, there is no indication that you should do any error handling before subsequent lines of code are executed.

Status Codes as Output Parameters

Let's also say an output parameter is used:

public class MerchantRepository
{
    public void Create(Merchant model, out MerchantStatusCode statusCode)
    {

        if (MerchantExists(model))
        {
            statusCode = MerchantStatusCode.MerchantAlreadyExists;
            return;
        }

        // Save to DB

        statusCode = MerchantStatusCode.Created;
    }
}

And the code to ignore the status code:

var merchantRepository = new MerchantRepository();
MerchantStatusCode code;

// Create a merchant. Sure I could check the "status" but, meh. If
// something fails it must not be THAT important. After all, good
// programmers are lazy, right?
merchantRepository.Create(merchant, out code);

While this exhibits more of a code smell, an error code doesn't halt program execution.

Now, let's tweak this to throw an exception:

Using Exceptions

public class MerchantRepository
{
    public void Create(Merchant model)
    {
        if (MerchantExists(model))
            throw new RecordExistsException();

        // Save to DB
    }
}

public class RecordExistsException : Exception
{
    ...
}

And the C# code to use this in the same manner as the previous example where the programmer blissfully ignores problems:

var merchantRepository = new MerchantRepository();

try
{
    merchantRepository.Create(merchant);
}
catch Exception
{
    // I realize this method throws exceptions when it fails to create
    // a merchant, but I am a complete idiot as a programmer, and
    // failure as a human being, so I'm going to blindly swallow any
    // possible errors, then move to another team or job and let you
    // try tracking down this production problem for 6 months while
    // the company bleeds customers and your boss screams at you so
    // hard he spits his bagel and coffee all over your face.
    //
    // But, hey. At least I didn't write "TODO: Error handling"!
}

Both examples work the same when the code is executed and a duplicate merchant is created, but there is one glaring difference here: The empty try-catch block. Empty try-catch blocks go beyond "code smells" to "steaming piles of rotting flesh that have been baking in the sun all afternoon." I get an ulcer every time I see this.

Now, if we tweak example #2 to be the exact same code as example #1:

var merchantRepository = new MerchantRepository();

// Even though I don't handle an exception, at least the next
// line of code won't execute if something goes terribly wrong.
// I'm not an idiot. I just HAVE to create a merchant here, and
// failure to do so is a hard stop.
merchantRepository.Create(merchant);

No subsequent lines of code will get executed, and the programmer does not have to guard against this condition unless they want to.

The lesson here: For the love of God do not blindly trap exceptions and do nothing, unless you document this behavior in comments and provide a really really really really really really really really really really really really really good reason for doing so.

Regarding your particular approach, there are a couple things that you should be doing instead:

Error Handling Best Practices Regarding Database Operations

Prior to creating a merchant, check the database to ensure the merchant doesn't already exist. This is basic data validation, and should be handled in its own layer.
When creating a merchant, if a merchant happens to exist, throw an exception which will force you, as a programmer, to handle this "exceptional situation" (hence the term Exception).

When a database operation is invoked, it needs to assume that all data is valid, and then throw an exception if anything goes wrong. Things should never be wrong in the first place, and the data access layer has no way of remedying this problem, so throwing an exception halts any further operation. Failing to create a merchant is a catastrophic, unresolvable issue requiring the immediate cessation of logic.

If attempting to create a duplicate merchant is something you can handle, then your Controller needs to trap this exception and handle it.

An interesting blog post about error codes and exceptions:

Error codes or Exceptions? Why is Reliable Software so Hard?

There can be a case for either, but I generally lean towards exceptions because they halt application execution, which is what you want most times.


Traffic light implementation Can someone review this code for me?  I'm not completely sure I did this correctly. I also would like help creating the code for the "walk" "don't walk" lights.

I needed to create a traffic intersection. Each traffic light has a cycle of 31 seconds red, followed by 26 seconds green, followed by 3 seconds yellow. The red in the north-south direction starts at the same time as the green in the east-west direction. There will be a 60 second light cycle, and a 2 second overlap where the lights are red in all directions. The walk-don't walk signal reads walk for the 26 second green light, and don't walk for all other times.

Public Class _12
Private Sub N_S_G_Lights()
    pb_N.BackColor = Color.Green
    lbl_N.BackColor = Color.Green
    pb_S.BackColor = Color.Green
    lbl_S.BackColor = Color.Green
End Sub
Private Sub N_S_Y_Lights()
    pb_N.BackColor = Color.Yellow
    lbl_N.BackColor = Color.Yellow
    pb_S.BackColor = Color.Yellow
    lbl_S.BackColor = Color.Yellow
End Sub
Private Sub N_S_R_Lights()
    pb_N.BackColor = Color.Red
    lbl_N.BackColor = Color.Red
    pb_S.BackColor = Color.Red
    lbl_S.BackColor = Color.Red
End Sub
Private Sub W_E_G_Lights()
    pb_W.BackColor = Color.Green
    lbl_W.BackColor = Color.Green
    pb_E.BackColor = Color.Green
    lbl_E.BackColor = Color.Green
End Sub
Private Sub W_E_Y_Lights()
    pb_W.BackColor = Color.Yellow
    lbl_W.BackColor = Color.Yellow
    pb_E.BackColor = Color.Yellow
    lbl_E.BackColor = Color.Yellow
End Sub
Private Sub W_E_R_Lights()
    pb_W.BackColor = Color.Red
    lbl_W.BackColor = Color.Red
    pb_E.BackColor = Color.Red
    lbl_E.BackColor = Color.Red
End Sub
Private Sub _12_Load(sender As System.Object, e As System.EventArgs) Handles MyBase.Load
    pb_N.BackColor = Color.Red
    pb_E.BackColor = Color.Red
    pb_S.BackColor = Color.Red
    pb_W.BackColor = Color.Red
    lbl_N.BackColor = Color.Red
    lbl_E.BackColor = Color.Red
    lbl_S.BackColor = Color.Red
    lbl_W.BackColor = Color.Red

End Sub

Private Sub Lights()
    If lbl_time.Text <= 33 Then
        W_E_R_Lights()
    ElseIf lbl_time.Text >= 34 And lbl_time.Text <= 58 Then
        W_E_G_Lights()
    ElseIf lbl_time.Text >= 59 Then
        W_E_Y_Lights()
    ElseIf lbl_time.Text >= 60 Then
        W_E_R_Lights()
    End If
    If lbl_time.Text <= 26 Then
        N_S_G_Lights()
    ElseIf lbl_time.Text >= 27 And lbl_time.Text <= 29 Then
        N_S_Y_Lights()
    ElseIf lbl_time.Text >= 30 Then
        N_S_R_Lights()
    End If
End Sub
Private Sub Timer_N_Tick(sender As System.Object, e As System.EventArgs) Handles Timer_N.Tick
    lbl_time.Text = Val(lbl_time.Text) + 1

End Sub

Private Sub Button1_Click(sender As System.Object, e As System.EventArgs) Handles Button1.Click
    lbl_time.Text = "55"
End Sub

Private Sub Button2_Click(sender As System.Object, e As System.EventArgs) Handles Button2.Click
    lbl_time.Text = "25"
End Sub

Private Sub Timer1_Tick(sender As System.Object, e As System.EventArgs) Handles Timer1.Tick
    If lbl_time.Text <= 60 Then
        Lights()
    Else
        lbl_time.Text = "0"
    End If
End Sub
End Class

Naming.
N_S_G_Lights could be called NorthSouthGreenLights, but that wouldn't make it a much better name. Method names should start with a verb, they do something.
However, the pattern is consistent, which is a good thing. It's redundant though.
Structure.
The things you need to do depend on the vocabulary you're writing your code with - methods are verbs, classes are subjects. Classes are definitions for objects, which encapsulate a piece of functionality and abstract away the implementation details.
The first object I'd think of a design for, would be some TrafficLight class. I'd want the UI code to only have to be told what color that traffic light is, and draw the appropriate background color in that PictureBox.
The form code doesn't need to know about a Timer - it needs to know about a TrafficLight, and from that object it needs to know the LightColor, and it needs to be notified whenever the color changes.
I'd be expecting something like this in a class called TrafficLight:
Private Const RedSeconds As Integer = 31
Private Const GreenSeconds As Integer = 26
Private Const YellowSeconds As Integer = 3

Private LightTimer As Timer 'ticks every second

Private CurrentColor As Color

Public Event Changed(ByVal NewColor As Color)

I like that you have a single timer for the whole 60-second cycle, but the magic numbers  in Lights() make it hard to correlate what you've done with what the requirements are. Using constants as above, helps making the code more readable. The fact that your logic relies on UI values isn't very nice, too.
When you have a TrafficLight that raises an event that says "Hey there, if it's of any interest to you, I'm changing my current color right now" to your form, the form itself doesn't know how the TrafficLight operates, but it knows that when color changes, it needs to change the background color of some PictureBox:
Private Sub NorthSouthLight_Changed(ByVal NewColor As Color)

    NorthTrafficLightPicture.BackColor = NewColor
    NorthLabel.BackColor = NewColor

    SouthTrafficLightPicture.BackColor = NewColor
    SouthLabel.BackColor = NewColor

End Sub

Private Sub EastWestLight_Changed(ByVal NewColor As Color)

    EastTrafficLightPicture.BackColor = NewColor
    EastLabel.BackColor = NewColor

    WestTrafficLightPicture.BackColor = NewColor
    WestLabel.BackColor = NewColor

End Sub

What color that is, isn't important - it's conveyed by the Changed event, if a TrafficLigth wanted to be Blue, the form would happily comply.

TL;DR: In other words, I think your code could greatly benefit from a little bit more of an Object-Oriented approach, especially if this code is vb.net (I was assuming vba all along, when I came across a Handles keyword, which I haven't seen used in VBA).


Singleton metaclass for each argument set compatible with Pickle I am creating a metaclass which ensures that instances of an actual class A are kind of singletons. But rather than having only single instance of A, I would like to have one instance per argument set given to A.
That is
class A(metaclass=ArgumentSingleton):
    pass

r = A()
s = A()
t = A(1)
u = A(1)
v = A('hi')

print(r is s)  # True
print(r is t)  # False
print(t is u)  # True
print(v is u)  # False
print(v is r)  # False

Moreover, the property of 'being a singleton' must survive pickling.
That is
a = A()

with open('tmp.pkl', 'wb') as f:
    pkl.dump(a, f)

with open('tmp.pkl', 'rb') as f:
    b = pkl.load(f)

print(a is b)  # True

For example Sympy is capable of doing that with their constants (such as pi) and I tried similar approach.
class ArgumentSingleton(type):
    _instances = {}

    def __new__(cls, name, bases, class_dict):
        def __init__(self, *args):
            self._args = args
            print('Here in __init__')

        def __reduce_ex__(self, protocol):
            return type(self), (*self._args,)

        init = '__init__'
        reduce = '__reduce_ex__'
        method_absence = [
            init not in class_dict,
            reduce not in class_dict
        ]

        if all(method_absence):
            class_dict[init] = __init__
            class_dict[reduce] = __reduce_ex__
        elif any(method_absence):
            raise ArgumentSingletonException(
                f"Either both methods '{init}', '{reduce}' are defined or "
                f"none of them in class: {name}"
            )

        new_class = super().__new__(cls, name, bases, class_dict)

        return new_class

    def __call__(cls, *args, **kwargs):
        key = (cls, *args, *kwargs.items())
        if key not in ArgumentSingleton._instances:
            ArgumentSingleton._instances[key] = super().__call__(*args, **kwargs)
        return cls._instances[key]

The code depends quite deeply (at least to my standards) on inner workings of Python and I am afraid of any hidden bugs and hidden problems.
The purpose of controling presence of __init__ and __reduce_ex__ is to explicitly move responsibility for __reduce_ex__ to the creators of the class A in case they decide to provide their __init__.
Any comments or suggestions appreciated!
So far, I am aware of the fact that the arguments must be hashable. That is not problem for me, as this construction is meant to speed up comparison of my complex hashable objects.

The point of having to define either neither or both of __init__ and __reduce_ex__ is to set _args.
However you can just set _args in __call__ and then use the one you've set in your __reduce_ex__.
Note: untested
class ArgumentSingleton(type):
    __INSTANCES = {}

    def __new__(cls, name, bases, class_dict):
        def __reduce_ex__(self, protocol):
            return partial(type(self), *self.__args, **self.__kwargs), ()

        def __init__(self, *args, **kwargs):
            pass

        class_dict.setdefault('__init__', __init__)
        class_dict.setdefault('__reduce_ex__', __reduce_ex__)
        return super().__new__(cls, name, bases, class_dict)

    def __call__(cls, *args, **kwargs):
        key = (cls, args, frozendict(kwargs))
        if None is (self := cls.__INSTANCES.get(key)):
            self = cls.__INSTANCES[key] = super().__call__(cls, *args, **kwargs)
            self.__args, self.__kwargs = key[1:]
        return self

Now we should be able to see we can just stop defining a metaclass by;

changing __call__ to __new__, and
defining a default __reduce_ex__ on the base class.

from frozendict import frozendict
from functools import partial

class Singleton:
    __INSTANCES = {}

    def __new__(cls, *args, **kwargs):
        key = (cls, args, frozendict(kwargs))
        if None is (self := cls.__INSTANCES.get(key)):
            self = cls.__INSTANCES[key] = super().__new__(cls)
            self.__args, self.__kwargs = key[1:]
        return self

    def __reduce_ex__(self, protocol: int):
        return partial(type(self), *self.__args, **self.__kwargs), ()

class A(Singleton):
    pass

# passes all your tests and the following one
a = A("hi")
b = pickle.loads(pickle.dumps(a))
assert a is b


Python proper abstract class and subclassing with attributes and methods The goal of the code below is to have an abstract base class that defines simple methods and attributes for the subclasses. This is part of an application that provides the code base for others to develop their own subclasses such that all methods and attributes are well implemented in a way for the main application to use them. The application shall never create more than one object of each subclass, since they are supposed to implement everything correctly to be called upon. Ideally, instantiating objects of the subclasses would not even be necessary, but is needed as it is in order to check if the abstract methods are implemented.

However, Python seems to be different and awkward when it comes to classes in comparison with other programming languages (initialization, attributes, properties), and I am not very sure if the solution below is the most appropriate. It seems too much complicated to achieve the aforementioned purpose. In addition, another function (not in the code below) must be made to properly check (make sure that they are of a certain type) the attributes of the subclasses.

Any thoughts?

from abc import ABC, abstractmethod

class abstract_attribute(object):
    """taken from http://stackoverflow.com/questions/32536176/how-to-define-lazy-variable-in-python-which-will-raise-notimplementederror-for-a/32536493#32536493"""
    def __get__(self, obj, type):
        # Now we will iterate over the names on the class,
        # and all its superclasses, and try to find the attribute
        # name for this descriptor
        # traverse the parents in the method resolution order
        for cls in type.__mro__:
            # for each cls thus, see what attributes they set
            for name, value in cls.__dict__.items():
                # we found ourselves here
                if value is self:
                    # if the property gets accessed as Child.variable,
                    # obj will be done. For this case
                    # If accessed as a_child.variable, the class Child is
                    # in the type, and a_child in the obj.
                    this_obj = obj if obj else type

                    raise NotImplementedError(
                         "%r does not have the attribute %r "
                         "(abstract from class %r)" %
                             (this_obj, name, cls.__name__))

        # we did not find a match, should be rare, but prepare for it
        raise NotImplementedError(
            "%s does not set the abstract attribute <unknown>", type.__name__)

class Base(ABC):
    # attr1 and attr2 must be properly (be of a certain type) implemented by subclasses
    # abstract_attribute() checks if the attribute is present in the subclass
    attr1 = abstract_attribute()
    attr2 = abstract_attribute()

    @abstractmethod
    def method():
        pass

    # a class property that does not change from instance to instance
    @property
    def prop(self):
        return self.prop
    @prop.setter
    @classmethod
    def prop(self, prop):
        self.prop = prop

class Sub(Base):
    # attr1 and attr2 must be implemented otherwise and exception is thrown upon accessing them
    attr1 = something
    attr2 = somethingElse
    # method must be implemented otherwise an exception is thrown upon instantiation
    def method():
        print("must be implemented by subclasses")

I don't see much of problems with using properties as abstract attributes e.g:

class Foo(abc.ABC):
    @property
    @abc.abstractmethod
    def bar(self):
        ...

However, if you still don't like it instead of going for your ninja stackoverflow way there is an easy way in python 3.6.

class Foo(abc.ABC):
    def __init_subclass__(cls, **kwargs):
        if not hasattr(cls, 'bar'):
            raise NotImplementedError('damn')
        return super().__init_subclass__(**kwargs)

As for one instance per class just look for python Singleton implementations.


Prime numbers with array and OpenMP in C #include <stdio.h>
#include <string.h>
#include <math.h>
#include <omp.h>

#define MAXN 10*1000*1000

int fillarray(unsigned maxn, int *p, int psize) {

    int i, top = 1;
    int n;
    p[0] = 3;
    printf(" 2\n 3\n");

    for (n = 5;; n += 2)

        for (i = 0; n % p[i] != 0; i++)

            if (p[i] * p[i] > n) {

                printf("%6d\n", n);
                p[top] = n;

                if (n * n > maxn)
                    return top;

                if (++top == psize) {
                    printf("Array too short\n");
                    return -1;
                }

                break;
           }
}

int main() {

    unsigned n, maxn = MAXN;
    int sum;

    /* Prime Number Theorem formula (x/logx) to find approx.
       size of array p[]. Needs some safety margin. */
    int psize =  10 + 1.2 * sqrt(maxn) / log(sqrt(maxn));
    int p[psize], *pp;

    int lastp = fillarray(maxn, p, psize);
    if (lastp < 0)
        return 1;

    /* "3" is p[0], plus the "2" */
    sum = lastp + 2;

    #pragma omp parallel for private(pp) schedule(guided) reduction(+:sum)

    for (n = p[lastp] + 2; n <= maxn; n += 2)

        for (pp = p; n % *pp > 0; pp++)

            if (*pp * *pp > n) {
                printf("%d\n", n);
                sum++;
                break;
            }

    printf("SUM %d (%.2f%%)\n", sum, (double) 100*sum / maxn) ;
    double lnx = maxn / log(maxn);
    printf("LnX %.2f (%f)\n",  lnx, sum / lnx);
    printf("psize %d\n", psize);
    printf("p[0] p[1]...p[%d] %d %d...%d\n", lastp, p[0], p[1], p[lastp]);

    return 0;
}

Compile with -fopenmp. And -lm for clang. But it works also without openmp.
Output:
# time ./a.out |tail
9999971
9999901
9999931
9999973
9999991
9999937
SUM 664579 (6.65%)
LnX 620420.69 (1.071175)
psize 480
p[0] p[1]...p[445] 3 5...3163

real    0m0.274s
user    0m1.497s
sys     0m0.402s

It takes 445 primes to reach 3163, whose square is > 10 million. The array size is 480. The output is not sorted, the threads print them directly.
The first (single-threaded) phase i.e. fillarray() takes no time compared to second, multi-threaded phase.
In main() the prime test is inlined, and written with pointers.

A couple of minor things.
For int psize, consider using size_t.
log(sqrt(maxn)) should simply be log(maxn)/2, which is equivalent.


Reduction of time for the code of sorting RGB and Depth images according to timestamp values I have RGB Images, Depth images and OpenPose Images(generated from RGB Images).
The number of RGB and Depth images are different so I have to find the exactly which RGB images corresponds to which depth images. This is done using the time stamp values I have for both RGB and Depth Images.

The problem here is this code I have written takes around 7-10 minutes to execute which I feel is a lot. How can the time be reduced for the same? Here mainly imwrite is taking lot of time.

function image_sorting(color_img, depth_img, Openpose, selected_color, selected_depth)

filePattern = fullfile(depth_img, '*.timestamp');
file = dir(filePattern);

filePattern2 = fullfile(color_img, '*.timestamp');
file2 = dir(filePattern2);

filePattern3 = fullfile(depth_img, '*.bmp');
file3 = dir(filePattern3);

filePattern4 = fullfile(Openpose, '*.png');
file4 = dir(filePattern4);

for k = 1:length(file)
    depthTimestampBase = file(k).name;
    depthTimestamp = fullfile(depth_img, depthTimestampBase);
    fileID = fopen(depthTimestamp,'r');
    % format longG
    A(k,:) = textscan(fileID,'%d64') ;
    fclose(fileID);
end

for m = 1:length(file2)
    rgbTimestampBase = file2(m).name;
    rgbTimestamp = fullfile(color_img, rgbTimestampBase);
    fileID2 = fopen(rgbTimestamp,'r');
    %format longG
    B(m,:) = textscan(fileID2,'%d64') ;
    fclose(fileID2);
end

%%%%%% Here there are two parts. Use any of them according to conditions
if length(file2) <= length(file)
    %PART 1:- If RGB images are less than depth then use the following code

    for m = 1:length(file2)
        for k = 1:length(file)

            C{k,m} = A{k,1} - B{m,1};
            if C{k,m}<0
                C{k,m} = -C{k,m};
            end
        end
    end

    [V,X] = min(cell2mat(C),[],1); % Is a row vector containing the minimum value of columns
    % V gives minimum value and X gives Index

    m = 1;
    for w= X

        depthDataBase = file3(w).name;
        rgbDataBase = file4(m).name;

        depthData = fullfile(depth_img, depthDataBase);
        rgbData = fullfile(Openpose, rgbDataBase);

        imageArrayy = imread(depthData);
        imageArrayy2 = imread(rgbData);

        depthData2 = fullfile(selected_depth, depthDataBase);
        imwrite(imageArrayy, depthData2);
        rgbData2 = fullfile(selected_color, rgbDataBase);
        imwrite(imageArrayy2, rgbData2);

        m = m+1; % for part 2 m = m+1
    end

    %
    % PART 2:- If RGB images are more than depth then use the following code
else

    for m = 1:length(file2)
        for k = 1:length(file)

            C{m,k} = A{k,1} - B{m,1};
            if C{m,k}<0
                C{m,k} = -C{m,k};
            end
        end
    end

    [V,X] = min(cell2mat(C),[],1); % Is a row vector containing the minimum value of columns
    % V gives minimum value and X gives Index

    w = 1;
    for m= X

        depthDataBase = file3(w).name;
        rgbDataBase = file4(m).name;

        depthData = fullfile(depth_img, depthDataBase);
        rgbData = fullfile(Openpose, rgbDataBase);

        imageArrayy = imread(depthData);
        imageArrayy2 = imread(rgbData);

        depthData2 = fullfile(selected_depth, depthDataBase);
        imwrite(imageArrayy, depthData2);
        rgbData2 = fullfile(selected_color, rgbDataBase);
        imwrite(imageArrayy2, rgbData2);

        w = w+1;
    end
end

Update 1:- Screenshot of Profile execution time.

You are reading in an image and then writing it back out again without making any changes. If the purpose is to copy the file, you are better off directly copying the file. You can use the function copyfile for that.

The code is fairly readable. There are some improvements you can make:

filePattern = fullfile(depth_img, '*.timestamp');
file = dir(filePattern);

filePattern2 = fullfile(color_img, '*.timestamp');
file2 = dir(filePattern2);

filePattern3 = fullfile(depth_img, '*.bmp');
file3 = dir(filePattern3);

filePattern4 = fullfile(Openpose, '*.png');
file4 = dir(filePattern4);

The four filePatternN variables are never used again. Why use four different variables, rather than repeat the same name? Why not omit the temporary variable and put these into single lines?

file = dir(fullfile(depth_img, '*.timestamp'));

Next, you have two identical loops, one reads from file, one from file2. One writes to A, one to B. Why not make this into a function? You can write local functions at the end of your M-file. You'd simplify this bit to:

A = get_time_stamps(file);
B = get_time_stamps(file2);

Another difference between these two loops is that one uses fileID and one uses fileID2. There is again no need to use two different names for local variables that you don't use outside the loop.

You can improve your code by making A and B numeric arrays rather than cell arrays:

tmp = textscan(fileID,'%d64');
A(k) = tmp{1};

Be sure to preallocate the arrays!

Next you have again two large blocks of mostly repeated code. if length(file2) <= length(file). You could put this into a function and call it do_stuff(file,file2) or do_stuff(file2,file) depending on which of the two is larger. But really, this might not be necessary at all with some better logic.

To find which time stamps are closest to each other, you don't need a O(n2) algorithm. You could instead sort the two arrays of time stamps, and walk through them one by one. The algorithm might be slightly more complex, but you wouldn't need to duplicate the code.

The other alternative is to use vectorized processing to do the O(n2) computation without using loops at all. But in modern versions of MATLAB, loops are not so slow any more, so this is not something you should be worrying about anyway. The way to speed up this loop is to preallocate the array C.

You can also simplify it by writing C{m,k} = abs(A{k,1} - B{m,1});, and removing the if statement.


Vectorizing a sequence \$x_{i+1} = f(x_i)\$ with NumPy I would like to populate a NumPy array with "accumulated" results from a custom function.

Currently, my code is:

import numpy as np
def f(x, mu):
    return mu * x * (1 - x)

def populate(x0, mu, n):
    s = np.zeros(n)
    x = x0
    for i in range(n):
        s[i], x = x, f(x, mu)
    return s

It does not take advantage of the vectorization performance of NumPy.

Is there any way to improve the speed of creating arrays like this?

You can't vectorize this. Repeated application of \$x_i = \mu x_{i-1} (1-x_{i-1})\$ is chaotic.


Comparing two simple distance calculating algorithms Is the single method printMaxDistance_a or the two methods printMaxDistance_b AND printMaxDistance_c better or respectively "more readable"? Thanks.

public static void printMaxDistance_a(String s) {
    int first = 0;
    int second = 0;
    int max = 0;
    for (int i = 0; i < s.length(); i++) {
        for (int j = i + 1; j < s.length(); j++) {
            if (s.charAt(i) == s.charAt(j)) {
                if ((j - i) > max) {
                    first = i;
                    second = j;
                    max = j - i;
                }
                break;
            }
        }
    }
    System.out.println(first + " " + second + " " + max);
}

public static void printMaxDistance_b(String s) {
    int first = 0;
    int second = 0;
    int max = 0;
    for (int i = 0; i < s.length(); i++) {
        int cur = printMaxDistance_c(s, s.charAt(i), i + 1);
        if (cur > max) {
            first = i;
            second = i + cur;
            max = cur;
        }
    }
    System.out.println(first + " " + second + " " + max);
}

public static int printMaxDistance_c(String s, char c, int j) {
    for (int i = j; i < s.length(); i++) {
        if (c == s.charAt(i)) {
            return i - (j - 1);
        }
    }
    return 0;
}

@SuppressWarnings("resource")
public static void main(String[] args) {
    String s = new Scanner(System.in).nextLine();
    printMaxDistance_a(s);
    printMaxDistance_b(s);
}

Not more readable with those method names, but in general it is better to split method into more, especially when code is nested like this. I would even go further and split into more than just 2 methods.

But try to name methods so that name clearly says, what method does.

Edit:

Some ideas of what I would change (starting right from _b and _c methods).

You probably know, that names should be "camelcase", use no underscores and you added underscore just to quickly separate those.
Method is calculating data and printing them. Those are 2 things at once. What if you wanted to use data instead of printing them? what if you wanted to write them to file or return as part of http response?

At first I would at least return String instead of printing it. Then you can do more things than just printing it. That would require renaming method as it doesn't print anymore.
That solution is still not ideal, because you are calculating 3 int numbers and instead you are returning String.
Instead you could create immutable class maybe called MaxStringDistance to contain those 3 numbers and instead of returning String, you return something like new MaxStringDistance(first, second, max).
Now you are missing String representation, that you previously wanted. One way would be to override toString of your new class so that you can then just print that object to get same results as original code or create another method, that will construct the String (I think within that object is best place for it).
You can argue, that method is now "constructing" your MaxStringDistance object. Then it makes sense to put this static method inside MaxStringDistance method as "factory method". Not sure about the name.
Parameter name "s" is short and not very descriptive, try to name variables so that it clearly states what does it contain. It's true, that in this case there's not much to say about it.

That is what I'd do regarding refactoring _b method just based on it's name and signature.

Now let's take a look at _c

This method is definitely not printing anything, but returning int. This one really needs renaming, probably something like calcualteCharDistance
Again single character parameters. I'd at least rename "j" to "position" because it seems like it represents character position in the string (+1 so that's why "position", not "index")
You probably won't be calling this method from anywhere else and can be set it's visibility to private.

Somewhat improved code will look like this:

public class MaxStringDistance {
    private final int first;
    private final int second;
    private final int max;

    //possibly private constructor
    public MaxStringDistance(int first, int second, int max) {
        this.first = first;
        this.second = second;
        this.max = max;
    }

    //getters if needed

    @Override
    public String toString() {
        return first + " " + second + " " + max;
    }

    public static MaxStringDistance fromString(String s) {
        int first = 0;
        int second = 0;
        int max = 0;
        for (int i = 0; i < s.length(); i++) {
            int cur = calculateCharDistance(s, s.charAt(i), i + 1);
            if (cur > max) {
                first = i;
                second = i + cur;
                max = cur;
            }
        }
        return new MaxStringDistance(first, second, max);
    }

    private static int calculateCharDistance(String s, char c, int position) {
        for (int i = position; i < s.length(); i++) {
            if (c == s.charAt(i)) {
                return i - (position - 1);
            }
        }
        return 0;
    }

    public static void main(String[] args) {
        String userInput = new Scanner(System.in).nextLine();
        System.out.println(MaxStringDistance.fromString(userInput));
    }
}


Vanilla JS/CSS -- set of buttons to trigger sliders by transitioning height value I've been working on a solution that allows a set of buttons to reveal divs that slide down from above on-click. I needed each button to toggle an active state for itself, but also if the user were to click on one button, reveal a div, and then go on to click another button, that the initially revealed div transition back up and dissapear while the new one appears.
When all the buttons are on one line, my solution works relatively well. But when there are buttons below some of the hidden divs, suddenly the divs those buttons trigger appear from below instead of dropping down from above... IF one of the buttons above has an active state... If not, business as usual.
Appologies if this is confusing, please see my fiddle : https://jsfiddle.net/maesj_/qdk4z1vm/26/
My solution is an adaptation of a demo that I found on css-tricks adressing a similar kind of situation, but in my case having mutliple buttons has added a bit more complexity to the mix and I'm wondering if anyone has any ideas on how I might improve my code.
Any thoughts are much appreciated!
HTML
<button  class="dyn-gallery-btn" type="button"> one </button>
<button  class="dyn-gallery-btn" type="button"> two </button>

 one text

 two text

<button  class="dyn-gallery-btn" type="button"> three </button>
<button  class="dyn-gallery-btn" type="button"> four </button>

 three text

 four text

CSS
.dyn-slide {
   background: red;
   transition: height .5s ease-in-out;
   height: 0;
   overflow: hidden;
}

.dyn-slide.active {
  height: auto;
  overflow: hidden;
}

.dyn-slide:not(.active) {
  height: 0;
  overflow: hidden;
}

JS
const slides = document.querySelectorAll('.dyn-slide')
const buttons = document.querySelectorAll('.dyn-gallery-btn')

buttons.forEach((button, i) => {
  button.addEventListener('click', () => {

    const slide = slides[i]
    const wasActive = slide.classList.contains('active')

    slides.forEach(slide => {
      slide.classList.remove('active')
      slide.style.height = 0;
    })

    if (!wasActive) {
      slide.classList.add('active')

      slide.style.height = 'auto';

    var height = slide.clientHeight + 'px';
    slide.style.height = '0px';

    setTimeout(function () {
      slide.style.height = height;
    }, 0);

    } else {
    var height = slide.clientHeight + 'px';
     slide.style.height = '0px';

     setTimeout(function () {
      slide.style.height = 0;
    }, 0);

    }
  })
})

Redundant CSS The rules in .dyn-slide:not(.active) already exist in the .dyn-slide block, so they're not needed - feel free to remove them entirely. (The .dyn-slide.active { height: auto; isn't being used either, since the height is being set directly via JS)
Semicolons or not? To be stylistically consistent and to reduce bugs, either use semicolons everywhere they're appropriate, or (if you consider yourself skilled enough not to run into problems with ASI) don't use semicolons. Choose a style you want, then enforce it.
Use const, avoid var - you're using modern syntax, which is great, but you're using var in a few places. In ES2015+, best to always use const (or, if you need to reassign, let), but var isn't a great idea - it has weird rules like function scope instead of block scope, and automatically assigns its variables to the global object when used on the top level, among other weirdness. (ESLint rule: no-var, I highly recommend using a linter to automatically prompt you to fix code quality smells)
Unused variable You do
} else {
  var height = slide.clientHeight + 'px';

but never use the height variable - remove it. (ESLint rule: no-unused-vars)
Whole else block is redundant Or, remove the whole block there: the element's height has been set to 0 at the beginning of the function, so setting it to 0 again won't change anything.
You can save the last active slide in a variable instead of looping over all slides at the beginning of the click handler.
Trigger reflow instead of using setTimeout Rather than setTimeout, you can trigger a DOM reflow by reading a style property from the DOM, for example, access the offsetHeight of an element. This allows you to code synchronously:
Indentation Some of your indented blocks are not properly aligned. Consider using an IDE to automatically indent correctly - this'll make it easier for you and others to understand the code at a glance.
Refactored:

const slides = document.querySelectorAll('.dyn-slide');
const buttons = document.querySelectorAll('.dyn-gallery-btn');
let lastSlide;

buttons.forEach((button, i) => {
  button.addEventListener('click', () => {
    const slide = slides[i];
    const wasActive = slide.classList.contains('active');
    if (lastSlide) {
      lastSlide.classList.remove('active');
      lastSlide.style.height = 0;
    }
    lastSlide = slide;
    if (wasActive) {
      return;
    }
    slide.classList.add('active');
    slide.style.height = 'auto';
    const newHeight = slide.clientHeight + 'px';
    slide.style.height = '0px';
    slide.offsetHeight; // Force a reflow
    slide.style.height = newHeight;
  })
})
.dyn-slide {
   background: red;
   transition: height .5s ease-in-out;
   height: 0;
   overflow: hidden;
}

.dyn-slide.active {
  /* Height will be set to either auto or 0 by JS */
  overflow: hidden;
}
<button  class="dyn-gallery-btn" type="button"> one </button>
<button  class="dyn-gallery-btn" type="button"> two </button>

 one text

 two text

<button  class="dyn-gallery-btn" type="button"> three </button>
<button  class="dyn-gallery-btn" type="button"> four </button>

 three text

 four text


Single segment length of a line I'm trying to get the distance traveled in a collection of routes on a straight line. The rule to determine the distance is that a route segment counts only once. For example given the segments (50, 70) and (60, 80), the total distance covered is 30 instead of 40, because the segment (60, 70) is covered twice.

Sample:

╔═══════╦═══════╦═══════╗
║ Route ║ From  ║  To   ║
╠═══════╬═══════╬═══════╣
║     1 ║   100 ║   110 ║
║     2 ║   115 ║   116 ║
║     3 ║   100 ║   111 ║
║     4 ║     0 ║    15 ║
║     5 ║    50 ║   110 ║
║     6 ║    70 ║   120 ║
╚═══════╩═══════╩═══════╝

In these routes the actual distance traveled was 85m. I made a solution that uses a bit array to control, and a counter to calculate the distance, with this solution the process speed and the memory consumption is good, up to 20250000 meters, after this distance the performance begins to fall. Could they recommend me some algorithm to solve this kind of problem?

Note: These distance are in a straight line, they are not routes that have georeference, they are segments of a straight line.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;
public class MainClass {
    static Estacion[] estaciones = new Estacion[]{
          new Estacion(100,110),
          new Estacion(115,116),
          new Estacion(100,111),
          new Estacion(0,15),
          new Estacion(50,110),
          new Estacion(70,120)
      };

  public static void Main (string[] args) {
      Console.WriteLine (Calcular());
  }

  public static int Calcular()
  {

    var min =estaciones.Min(m=> m.Desde);
    var max =estaciones.Max(m=> m.Hasta);

    var recorrido =0;
    var cantidad= max-min;
    var bA = new BitArray(cantidad,false);
    foreach(var e in estaciones.OrderByDescending(o=> o.Longitud))
    {
      var h= ((e.Hasta-e.Desde)/2);
      for(int i= 0;i<= h ; i++)
      {
        var slot=i+e.Desde-min;
        if(slot<cantidad)
        if(!bA.Get(slot))
        {
           bA.Set(slot,true);
          recorrido++;
        }
        slot=e.Hasta-i-min-1;
        if(!bA.Get(slot))
        {
           bA.Set(slot,true);
          recorrido++;
        }
        if(recorrido== cantidad)
          return recorrido;
      }
    }
    return recorrido;
  }

  public class Estacion
  {
    public Guid Id {get; set;}
    public int Desde {get; set;}
    public int Hasta {get; set;}
    public int Longitud {get; set;}

    public Estacion(int a, int b)
    {
      Desde = a;
      Hasta= b;
      Longitud= Hasta-Desde;
      Id= Guid.NewGuid();
    }
  }
}

Algorithm

The time and space complexity of the current algorithm depends on the length of intervals in the input. This is very inefficient for inputs with long intervals, even when there are only few intervals.

The problem can be reworded as: merge the intervals and calculate their total length.
This should be fairly simple to implement efficiently:

Sort the intervals by start position ascending, and end position descending. That is, (5, 10) should come before (5, 8).
Initialize the previous interval = the first, and total = 0
Iterate from the second interval until the end, at each step:

If the current interval is fully included in previous, then skip it
If the current interval overlaps with previous, then update or replace previous to fully include both the previous and the current
If the current interval doesn't overlap with previous, then add the length of previous to total, and replace previous with the current

At the end, add the length of previous to total

Readability

There are serious readability concerns with this code.
This writing style is error-prone and confusing:

var slot=i+e.Desde-min;
if(slot<cantidad)
if(!bA.Get(slot))
{
   bA.Set(slot,true);
  recorrido++;
}
slot=e.Hasta-i-min-1;

The above should have been written like this:

var slot = i + e.Desde - min;
if (slot < cantidad)
{
    if (!bA.Get(slot))
    {
        bA.Set(slot, true);
        recorrido++;
    }
}
slot = e.Hasta - i - min - 1;

This way the scope of the if statements is perfectly clear.

I also added spaces around operators and after commas as per the convention.

As a side note, in your algorithm the slot < cantidad condition is always true, so in this example the first if statement had no effect,
as if it wasn't even there.


Brainfuck Printer Generator Given a text, this program will output a Brainfuck program, that when executed will print the text back.

  Writing Brainfuck in this style to print a sentence is pretty straightforward. Iterate over the sentence and do the following per character:

  Calculate the difference between the current character and the previous
  Write either as many + or - as the difference between characters. + if current character is higher than previous character, - if lower than previous character
  Write . to print current character

Hello, World!

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++.
+++++++++++++++++++++++++++++.
+++++++.
.
+++.
-------------------------------------------------------------------.
------------.
+++++++++++++++++++++++++++++++++++++++++++++++++++++++.
++++++++++++++++++++++++.
+++.
------.
--------.
-------------------------------------------------------------------.

The code

def brainturing_printer_generator(text)
  ords = text.chars.map(&:ord)
  ("+" * ords.first + ".\n") + ords
    .zip(ords[1..-1])
    .take(ords.size - 1)
    .map {|prev, curr| (curr > prev ? "+" : "-") * (curr - prev).abs}
    .join(".\n") + ".\n"
end

print brainturing_printer_generator("Hello, World!")

It's neat, though I feel like there's a bit too much array manipulation going on.

Your friend for this, I'd argue, is the each_cons method, short for "each consecutive". Then you don't need the zip, though you do need to pad the beginning of the array.

Here's my take:

def brainturing_printer_generator(text)
  ords = [0] + text.chars.map(&:ord)
  ords.each_cons(2).map do |previous, current|
    delta = current - previous
    line = (delta < 0 ? "-" : "+") * delta.abs
    "#{line}."
  end.join("\n")
end


UrlRouter which uses regular expressions to find out what to do I have created UrlRouter. It should call appropriate controllers methods with extracted values as a parameters. It works fine, but my boss told me that I should make it better. Could you give me some advice on how to improve this class, please?

public class UrlRouter {

    private final static String pageRegex = "/page/\\d{1,5}";
    private final static String yearRegex = "/\\d{4}";
    private final static String monthRegex = "/\\d{1,2}";
    private final static String nameRegex = "/[a-z0-9\\-]+";

    private final static Pattern page = Pattern.compile(pageRegex + "$");
    private final static Pattern year = Pattern.compile(yearRegex + "$");
    private final static Pattern yearPage = Pattern.compile(yearRegex + pageRegex + "$");
    private final static Pattern yearMonth = Pattern.compile(yearRegex + monthRegex  + "$");
    private final static Pattern yearMonthPage = Pattern.compile(yearRegex + monthRegex + pageRegex  + "$");
    private final static Pattern yearMonthName = Pattern.compile(yearRegex + monthRegex + nameRegex  + "$");
    private PostController postController;

    public UrlRouter(PostController controller) {
        this.postController = controller;
    }

    public void route(HttpServletRequest request) {
        String url = request.getRequestURI();

        if (yearMonthName.matcher(url).find()) {
            showOne(url);
        } else if (yearMonthPage.matcher(url).find()) {
            showMonthList(url);
        } else if (yearMonth.matcher(url).find()) {
            showMonth(url);
        } else if (yearPage.matcher(url).find()) {
            showYearList(url);
        } else if (year.matcher(url).find()) {
            showYear(url);
        } else if (page.matcher(url).find()) {
            showPage(url);
        } else {
            postController.showPage(1);
        }
    }

    private void showPage(String url) {
        String[] parts = url.split("/");
        int pageNumber = Integer.parseInt(parts[parts.length-1]);
        postController.showPage(pageNumber);
    }

    private void showYear(String url) {
        String[] parts = url.split("/");
        int year = Integer.parseInt(parts[parts.length - 1]);
        postController.showYear(year, 1);
    }

    private void showYearList(String url) {
        String[] parts = url.split("/");
        int year = Integer.parseInt(parts[parts.length - 3]);
        int page = Integer.parseInt(parts[parts.length - 1]);
        postController.showYear(year, page);
    }

    private void showMonth(String url) {
        String[] parts = url.split("/");
        int month = Integer.parseInt(parts[parts.length - 1]);
        int year = Integer.parseInt(parts[parts.length-2]);
        postController.showMonth(year, month, 1);
    }

    private void showMonthList(String url) {
        String[] parts = url.split("/");
        int year = Integer.parseInt(parts[parts.length - 4]);
        int month = Integer.parseInt(parts[parts.length - 3]);
        int page = Integer.parseInt(parts[parts.length - 1]);
        postController.showMonth(year, month, page);
    }

    private void showOne(String url) {
        String[] parts = url.split("/");
        int year = Integer.parseInt(parts[parts.length - 3]);
        int month = Integer.parseInt(parts[parts.length - 2]);
        String name = parts[parts.length - 1];
        postController.showOne(year, month, name);
    }
}

The first improvement you definitely want is capturing groups. These groups allow you to get rid of splitting at "/" and working on magic indices and instead use a Matcher to extract the parts from a matched string.

This greatly simplifies your delegating methods that work on a String to using the Matcher instance you used and threw away in your if-condition:

private void showPage(Matcher url) {
    postController.showPage(url.group(1));
    // alternatively use
    postController.showPage(Integer.parseInt(url.group(1)));
}

This would only require minor changes for your regexes:
You'd need to enclose the captured groups in parentheses as follows.

private final static String pageRegex = "/page/(\\d{1,5})";
private final static String yearRegex = "/(\\d{4})";
private final static String monthRegex = "/(\\d{1,2})";
private final static String nameRegex = "/([a-z0-9\\-]+)";


Find the year when the user will turn 100 I moved to Python today after doing a little of c++. The following code works as intended, but I want to know if I should add the if __name__ == "__main__" statement. Any other improvements?

import datetime

def inputInfo() :
    userName = input("Who are you?")
    userAge = input("How old are you?")
    return (userName , userAge)

def findYearWhenAge100(name , age) :
    currentTime = str(datetime.datetime.now())
    currentYear = int(currentTime[0: 4])
    age = 100 - int(age)
    currentYear += age
    print( name ,"is going to be 100 by", currentYear)

if __name__ == "__main__":
    userName , userAge = inputInfo()
    print(findYearWhenAge100(userName , userAge))

Your findYearWhenAge100() function has a print() statement, and returns nothing, so your __main__ doesn’t need to print the return value.

A good habit is adding """docstrings"""  to your functions.  Eg)

def inputInfo():
    """Asks the user for name and age"""
    #... remainder of code

The userInput() function should do things like turning the userAge string into an int.  That should not be the responsibility of functions which need the age.

Use pylint.  It will help you write more standardized code.  Things like a space is expected after a comma, not before.


Checking for all query parameters before inserting into a database The code works as it is but it's very messy. Is there a way to clean up the null checking in the if statement?

public function submitCoil ($data) {
    $db = Database::getInstance();

    //Define certain query values
    $uniqueID = $this->hash("id");
    $queryData = array(
        'fields' => array(
            'uniqueid',
            'uploaded',
            'name',
            'resistance',
            'wrapped',
            'category',
            'description',
            'wraps',
            'images',
            'wire_one',
            'wire_two',
            'wire_three',
            'wire_four',
            'wire_five',
            'wire_six'
        ),
        'values' => array(
            'uniqueID' => $uniqueID,
            'uploaded' => date("F j, Y", time()),
            'name' => $data['name'],
            'resistance' => $data['resistance'],
            'wrapped' => $data['wrapped'],
            'category' => $data['category'],
            'description' => $data['description'],
            'wraps' => $data['wraps'],
            'images' => "coils/" . $uniqueID . "/",
            'wire_one' => $data['wire_one'],
            'wire_two' => $data['wire_two'],
            'wire_three' => $data['wire_three'],
            'wire_four' => $data['wire_four'],
            'wire_five' => $data['wire_five'],
            'wire_six' => $data['wire_six']
        )
    );

    //Make sure mandatory datas are filled out
    if (!empty($queryData['values']['name']) && !empty($queryData['values']['resistance']) && !empty($queryData['values']['wrapped']) && !empty($queryData['values']['description']) && !empty($queryData['values']['wraps'])) {
        $db->insertFew("coils", $queryData['fields'], $queryData['values']);
    } else {
        echo "Please fill all fields";
    }

Use a validation function
There are many possible solutions, one thing you can do is use a separate function to validate your data:
public function submitCoil($data)
{
    ...
    if (!$this->validates($queryData['values'])) {
        return false;
    }

    return $db->insertFew("coils", $queryData['fields'], $queryData['values']);
}

protected function validates($values)
{
    $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];

    $return = true;

    foreach ($mandatoryFields as $field) {
        if (empty($values[$field])) {
            // Consider doing something like this so you can know/say
            // what the problem is.
            // $this->validationErrors[$field] = "This is empty";
            $return = false;
        }
    }

    return $return;
}

This function simply loops on all the keys that you specify as needing to be not-empty and returns true only if they are all populated.
Many simple functions
Always strive to have simple functions, that do one thing - for example:
public function submitCoil($data)
{
    $queryData = $this->translateData($data);
    if (!$this->validates($queryData['values'])) {
        return false;
    }
    return $this->insert($queryData);
}

protected function translateData($data)
{
    $uniqueID = $this->hash("id");
    return [
        'fields' => [
            'uniqueid',
            'uploaded',
            'name',
            'resistance',
            'wrapped',
            'category',
            'description',
            'wraps',
            'images',
            'wire_one',
            'wire_two',
            'wire_three',
            'wire_four',
            'wire_five',
            'wire_six'
        ],
        'values' => [
            'uniqueID' => $uniqueID,
            'uploaded' => date("F j, Y", time()),
            'name' => $data['name'],
            'resistance' => $data['resistance'],
            'wrapped' => $data['wrapped'],
            'category' => $data['category'],
            'description' => $data['description'],
            'wraps' => $data['wraps'],
            'images' => "coils/" . $uniqueID . "/",
            'wire_one' => $data['wire_one'],
            'wire_two' => $data['wire_two'],
            'wire_three' => $data['wire_three'],
            'wire_four' => $data['wire_four'],
            'wire_five' => $data['wire_five'],
            'wire_six' => $data['wire_six']
        ]
    ];
}

protected function validates($values)
{
    $mandatoryFields = ['name', 'resistance', 'description', 'wraps'];

    foreach ($mandatoryFields as $field) {
        if (empty($values[$field])) {
            return false;
        }
    }

    return true;
}

protected function insert($data)
{
    $db = Database::getInstance();
    return $db->insertFew("coils", $data['fields'], $data['values']);
}

This allows the building-blocks to be changed - or used individually. It also aides readability/maintainability as it is much clearer at a glance what a function does (assuming methods are well-named).
Avoid repetition
The fields and array keys for the values key in $queryData look to serve the same purpose - although they are different. You don't need 'fields' if the keys can be used i.e.:
$queryData = [
    'uniqueid' => $uniqueID,
    'uploaded' => date("F j, Y", time()),
    'name' => $data['name'],
    'resistance' => $data['resistance'],
    'wrapped' => $data['wrapped'],
    'category' => $data['category'],
    'description' => $data['description'],
    'wraps' => $data['wraps'],
    'images' => "coils/" . $uniqueID . "/",
    'wire_one' => $data['wire_one'],
    'wire_two' => $data['wire_two'],
    'wire_three' => $data['wire_three'],
    'wire_four' => $data['wire_four'],
    'wire_five' => $data['wire_five'],
    'wire_six' => $data['wire_six']
];

$db->insertFew("coils", array_keys($queryData), array_values($queryData));

This alone makes the method body much shorter.


C++ wrapper for font opaque pointer I'm creating a class which has a raw pointer member which comes from a C function. In order to make it RAII approved, I just created a method free() that is called on the destructor of this class.
Reading some guidelines I came across that Rule of 5 (and Rule of 3) which, from my understanding, I should also create a bunch of constructors and operator overloadings.
This is what I've done:
/* header */
class SDL2_Font {
 public:
  SDL2_Font() {}
  SDL2_Font(const SDL2_Font& other);  // copy constructor
  SDL2_Font(SDL2_Font&& other);       // move constructor
  ~SDL2_Font() { free(); }

  SDL2_Font& operator=(const SDL2_Font& other); // copy assignment
  SDL2_Font& operator=(SDL2_Font&& other);      // move assignment

  bool loadFont(const std::string& path, int size);
  // more stuff
 private:
  void free();

  TTF_Font* font_ = nullptr;
  FontInfo font_info_ {};
  std::string path_ {};
  int size_ = 0;
}

/* cpp file */
SDL2_Font::SDL2_Font(const SDL2_Font& other) {
  if (other.font_ != nullptr) {
    SDL2_Font::loadFont(other.path_, other.size_);
  }
}

SDL2_Font::SDL2_Font(SDL2_Font&& other): font_{other.font_} {
  other.font_ = nullptr;
}

SDL2_Font& SDL2_Font::operator=(const SDL2_Font& other) {
  if (&other != this && other.font_ != nullptr) {
    SDL2_Font::loadFont(other.path_, other.size_);
  }
  return *this;
}

SDL2_Font& SDL2_Font::operator=(SDL2_Font&& other) {
  if (&other != this) {
    SDL2_Font::free();
    font_ = other.font_;
    other.font_ = nullptr;
  }
  return *this;
}

void SDL2_Font::free() {
  if (font_ != nullptr && TTF_WasInit()) {
    TTF_CloseFont(font_);
    font_ = nullptr;
    path_.clear();
    size_ = 0;
    font_info_ = {};
  }
}

bool SDL2_Font::loadFont(const std::string& path, int size) {
  SDL2_Font::free();
  font_ = TTF_OpenFont(path.c_str(), size);
  if (font_ == nullptr) {
    ktp::logSDLError("TTF_OpenFont");
    return false;
  }
  path_ = path;
  size_ = size;
  SDL2_Font::queryFontInfo();
  return true;
}

This works, or at least I looks to me. But because it's my first time doing this stuff I just want to somebody take a look at it. What do you think? Is the way I implemented the methods/overloads correct?
Or maybe it's a bit overkill? Because I don't intend to copy/move/assign SDL2_Fonts and just for that destructor I had to do all those methods.

Because you

don't intend to copy/move/assign SDL2_Fonts

you could formalize that with
SDL2_Font(const SDL2_Font& other) = delete;
SDL2_Font& operator=(const SDL2_Font& other) = delete;

(the move versions of these operations won't be generated if these two are defined or deleted).
However, let's assume that one day we will want to be able to pass these fonts by value.
The code looks correct to my eye, but you could avoid having to do the memory management yourself if you use a smart pointer instead of a raw pointer for the font_ member.  If you do that, you can follow the Rule of Zero (the best of these rules) and simply let the compiler create the appropriate copy operators and constructors - see worked code below.
I notice that free() can only ever be called from the destructor. (Edit - I see it's also called from loadFont() but less obviously so, by being class-qualified.  This paragraph still applies, though.) free() does a lot of "dead writes" most times it's called - i.e. making assignments to variables that can never be read.  That's a waste of your time and mine!  Possibly also of your processor's, though a decent compiler should be able to optimise most of that away.

Here's a simplified version using shared pointer (I'm assuming that copies don't each need their own font object, as we don't ever modify it):
#include <SDL.h>
#include <SDL_ttf.h>

#include <memory>
#include <string>

namespace ktp {
    void logSDLError(const char*);
}

/* header */
class SDL2_Font {
public:
    SDL2_Font() {}

    bool loadFont(const std::string& path, int size);
private:
    struct FontInfo {};
    void queryFontInfo();

    std::shared_ptr<const TTF_Font> font_{};
    FontInfo font_info_ {};
    std::string path_ {};
    int size_ = 0;
};

// look, no constructors or assignment operators!

bool SDL2_Font::loadFont(const std::string& path, int size)
{
    font_.reset(TTF_OpenFont(path.c_str(), size),
                &TTF_CloseFont);
    if (!font_) {
        ktp::logSDLError("TTF_OpenFont");
        font_info_ = {};
        path_ = {};
        size_ = 0;
        return false;
    }
    path_ = path;
    size_ = size;
    queryFontInfo();
    return true;
}

See how reset() takes care of releasing resources for us, and we don't need to write a destructor.
I tried this with a simple main():
int main()
{
    if (TTF_Init()) {
        std::cerr << "TTF_Init: " << TTF_GetError() << '\n';
        return 1;
    }

    {
        SDL2_Font f;
        f.loadFont("/usr/share/fonts/truetype/unifont/unifont.ttf", 14);
        f.loadFont("/usr/share/fonts/truetype/unifont/unifont.ttf", 16);
        // important - f goes out of scope before TTF_Quit()
    }

    TTF_Quit();
}

This runs cleanly under Valgrind:
==29172== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==29172== Command: ./255607
==29172==
==29172==
==29172== HEAP SUMMARY:
==29172==     in use at exit: 0 bytes in 0 blocks
==29172==   total heap usage: 155 allocs, 155 frees, 615,577 bytes allocated
==29172==
==29172== All heap blocks were freed -- no leaks are possible