WOMOIWIW: What Org-mode Opens is What I Want Motivation
Previously, on the Emacs stack exchange:

In Org mode, when I open a link (C-c C-o) [...] [that] contains a wildcard, such as file:3_o*.pdf, Emacs opens it through dired instead [...] within Emacs.
I would instead have it open the first match of that pattern in the system application …

The handling of file: links is hard-coded in Org mode. If there is any wildcard in the given path, then org-link-open will nope out of the situation and instead call dired. And while one could change Org mode's source code in the local elisp files, I remembered that you can change the arguments or results of functions with advices.
Solution
I opted for an advice :around org-link-open with the following code:
(defun my-org-expand-file-link (link)
  "Expand a pattern in LINK to the first matching file.

Returns the original LINK if no file matches the pattern in LINK or if it is not
a link of type 'file:'.  See info node `(org)External links' for more information."
  (let ((type (org-element-property :type link))
        (path (org-element-property :path link)))
    (cond
     ((equal type "file")
      (let ((candidates (file-expand-wildcards path)))
        (if candidates
            (org-element-put-property (org-element-copy link) :path (car candidates))
          link)))
     (t link))))

(defun my-org-link-open-advice (orig-fun link &rest args)
  "Advice for ORIG-FUN that expands patterns in 'file:' LINKs.
See `my-org-expand-file-link' for more information.
Optional argument ARGS are passed as-is."
  (apply orig-fun (my-org-expand-file-link link) args))

(advice-add 'org-link-open :around #'my-org-link-open-advice)

So whenever you open a link with C-c C-o (org-open-at-point which calls org-link-open), you end up with a concrete file.

Some remarks
I recommend using a custom link type like first: or wrapping the code above minor mode, as the advice is somewhat invasive and might surprise users that really want to get a list of matching files instead.
I'm open to any feedback on the elisp code, especially since I'm still a elisp newbie.

That's a cool feature I've to say.  And the code looks excellent, I
suppose you could argue for one or the other wrt. cond vs. if
etc. but that's pointless pedantry IMO.
Given that it works flawlessly (tried locally and via tramp as well), I
was unsure there was actually anything to say here, but I found two
points that might be of interest:
Firstly, while the :around combinator works fine, it might be more
appropriate to use :filter-args, since it's really just filtering the
arguments:
(defun my-org-link-open-advice (args)
  "Advice for ORIG-FUN that expands patterns in 'file:' LINKs.
See `my-org-expand-file-link' for more information.
Optional argument ARGS are passed as-is."
  (list* (my-org-expand-file-link (first args)) (rest args)))

(advice-add 'org-link-open :filter-args #'my-org-link-open-advice)

The functionality is absolutely unchanged of course.
Secondly, and this is conflicting with the previous point, I could
imagine using the prefix argument (so C-u C-c C-o) to
distinguish the two cases of whether to run the original behaviour, or
not, like so:
(defun my-org-link-open-advice (orig-fun link &optional arg &rest args)
  "Advice for ORIG-FUN that expands patterns in 'file:' LINKs.
See `my-org-expand-file-link' for more information.
Optional argument ARGS are passed as-is."
  (apply orig-fun (if arg link (my-org-expand-file-link link)) arg args))

To quote the function docstring for org-link-open:

ARG is an optional prefix argument.  Some link types may handle it.
For example, it determines what application to run when opening a
"file" link.

This might interfere with the above idea, however practically I couldn't see a
difference with the unadvised function and this argument being present
or not, YMMV.


Upload image using php 7 best practices to make it secure I have a script that uploads an image, inserts it into database,  and displays it on the home page.

However it's not secure, how can i make this script more secure and make sure it allows only jpeg/jpg files.

UploadController.php

public function upload_image($filename, $image_name)
{

    $this->uploadfile = $this->directory . basename($filename);

    try{
        $sql = "INSERT INTO images (img, image_name) VALUES (?, ?)";
        $stmt = $this->connect->prepare($sql)->execute([$filename, $image_name]);
        return $stmt;
    }
    catch(PDOExeception $e)
    {
        echo $e->getMessage();
    }

}

Home.php

<?php
require_once 'paritals/header.php';
require_once 'paritals/nav.php';

use Eli\Db as DB;
use ElCont\UploadController as Image;

    if(isset($_FILES['profile_img'])){

        $dbh = new DB();
        $connect = $dbh->connect();
        $image = new Image($connect);

        $filename = $_FILES["profile_img"]["name"];
        $realname = "public/uploads/" . basename($filename);
        $directory = $_SERVER['DOCUMENT_ROOT'] . "/public/uploads/";

        $path = $directory . basename($filename);

        $image_name = $_POST['image_name'];

        if($image->upload_image($realname, $image_name)){

            move_uploaded_file($_FILES["profile_img"]["tmp_name"], $path);

            $owl = $image->get_image();

        }

}?>

Upload Image

            <form action="" enctype="multipart/form-data" method="post">

                    File input
                    <input type="file" name="profile_img" class="form-control-file" id="exampleInputFile" aria-describedby="fileHelp">
                    <input type="text" name="image_name" placeholder="Enter Name Of File" class="mt-3">
                    This is some placeholder block-level help text for the above input. It's a bit lighter and easily wraps to a new line.
                    <button type="submit" class="btn btn-primary">Submit</button>

            </form>

<?php

$dbh = new DB();
$connect = $dbh->connect();
$image = new Image($connect);

$photo =  $image->get_images();
foreach($photo as $pic)

{?>

<?php echo $pic['image_name'];?>

                <img src="<?php echo $pic['img'];?>" width="300" height="400">

<?php }?>

<?php require_once  'paritals/footer.php';?>

For some reason you aren't following suggestions have been given to you in the other post. So again:

One of insecurities is revealing the internal error message to a site user. Remove that try and catch stuff from the database interaction and configure your server to report errors appropriately like you've been told.

Same goes for adding directory to the file name. Please remember, you already added it in the step before:

$realname = "uploads/" . basename($filename);

therefore, the following code makes no sense:

$this->uploadfile = $this->directory . basename($filename);

Do you realize that?
Besides, a function responsible for storing a filename in the database should be dong only that - storing a filename. Assigning the filename or error reporting should be done elsewhere.

It means your upload image method should be exactly like this:

public function store($filename, $image_name)
{
    $sql = "INSERT INTO images (img, image_name) VALUES (?, ?)";
    $this->connect->prepare($sql)->execute([$filename, $image_name]);
}

making it to do the only stuff it is intended for.

Regarding security, the main point is to verify the file's extension. Your web-server will judge the file type by its extension so should you.

$allowed = ['jpg', 'jpeg', 'png', 'gif'];
$extension = pathinfo($_FILES['file']['name'],PATHINFO_EXTENSION);
if (in_array($extension, $allowed)) {
    // process your file
} else {
    // tell a user the extension is not allowed
}

regarding general purpose best practices. Looks like your namespaces are inconsistent. It seems you want something like that

use Eli\Db as DB;
use Eli\Controller\UploadController as Image;

and also you are supposed to implement PSR-4 autoload so you won't have to include class definitions manually


Finding a string whose MD5 starts like the digits of π I tried to make a program to find a string whose MD5-hash starts like the digits of pi, where dot is omitted. Is there a faster way to do it than this:

import hashlib
import random
import string
n = 1
while True:
    word = ''.join(random.choice(string.ascii_lowercase) for _ in range(20))
    if (str(hashlib.md5(word.encode('utf-8')).hexdigest()))[:n] == '31415926535897932384626433832795'[0:n]:
        print('n='+str(n)+", "+word)
        n = n + 1

The algorithm you're using is a brute-force algorithm.  It will require around (24)n hash checks for a match of n digits.  Each additional digit will require an average of 16-1 more time to calculate than the previous one.

The first, and hopefully obvious, step would be to get a better algorithm for a huge speedup - unfortunately, this is non-trivial in this case.

An additional step could be to use a language that is, typically, faster than Python.  You probably want a compiled language like C/C++, or even Java.

Lastly, this is a perfect parallel computing problem.  You have lots of small pieces (i.e. pick some text and hash it) and each is distinct, so they're easy to do in parallel.

Here I've given some improvements for a Python program, brute-force algorithm, singly-threaded.

The most important aspect of the code is how quickly we can create the word and calculate the md5 hash.  I've rewritten your code to not use an infinite loop, but instead loop for a specific number of tries.  This lets us time things using timeit.

from hashlib import md5
from random import choice, shuffle
from string import ascii_lowercase
from timeit import timeit
from itertools import product, islice
from os.path import commonprefix

def find_hash_pi_external(tries):
    n = 1
    for _ in range(tries):
        word = ''.join(choice(ascii_lowercase) for _ in range(20))
        if (str(md5(word.encode('utf-8')).hexdigest()))[:n] == '31415926535897932384626433832795'[0:n]:
            print('n=' + str(n) + ", " + word)
            n = n + 1

Some other comments:

The method hexdigest() returns a string, so you don't need to call str() there.
You can exclude the 0 in a[0:n].
You don't always have to increment n by one - sometimes you've found one that will match more than n characters.

def find_hash_pi_1(tries):
    pi = '31415926535897932384626433832795'
    n = 1
    for _ in range(tries):
        word = ''.join(choice(ascii_lowercase) for _ in range(20))
        hash = md5(word.encode('utf-8')).hexdigest()
        if hash[:n] == pi[:n]:
            n = len(commonprefix([pi, hash])) # note: commonprefix is slow so we only use it if there is a success
            print('n=' + str(n) + ", word=" + word + ", hash=" + hash)
            n = n + 1

The code above calls choice many times to make each word random.  We don't need to do that - instead, we need only different word value in the each step of the loop. It also creates the word as a string, but the md5 module requires bytes.  It is expensive to convert, so we can try to create the word as bytes without the string intermediate.

def find_hash_pi_2(tries):
    pi = '31415926535897932384626433832795'
    n = 1
    letters = bytes(ascii_lowercase, 'utf-8')
    for word in islice(product(letters, repeat=20), tries):
        word = bytes(word)
        hash = md5(word).hexdigest()
        if hash[:n] == pi[:n]:
            n = len(commonprefix([pi, hash])) # note: commonprefix is slow so we only use it if we have a hit
            print('n=' + str(n) + ", word=" + word.decode('utf-8') + ", hash=" + hash)
            n += 1

At this point the profiler is telling me that the built-in method _hashlib.openssl_md5 takes up about 10% of our time.  There is still space to improve, but our brute-force algorithm that uses separate md5() calls will be difficult to improve by much more.

Below I've shown the timeit output for all three solutions

if __name__ == "__main__":
    print(timeit('find_hash_pi_external(2 ** 16)', setup="from __main__ import find_hash_pi_external", number=1))
    print(timeit('find_hash_pi_1(2 ** 16)', setup="from __main__ import find_hash_pi_1", number=1))
    print(timeit('find_hash_pi_2(2 ** 16)', setup="from __main__ import find_hash_pi_2", number=1))

Results:

n=1, iaathglhsnzablgilhim
n=2, vjvcpilnwymbckhubdxl
n=3, gteyvjexqwahhbgasvco
n=4, rhjccvcubaeqfurjbfgi
1.227025089520841
n=1, word=wanforjhvqrpgnbbvlap, hash=3feca550be69a201c6147a0a20dbf338
n=2, word=eulvqycsfrgqhotpqsah, hash=3177a30c82a3b12856def16568d8b155
n=3, word=knmwruvaahghowwsyqyx, hash=3146373e28453b9dc7d1bb9961917dec
n=4, word=qrsgbfrcwrzwlqjgmryp, hash=31414f71ee6ba9516c9d0acb16e67662
1.239360037012343
n=1, word=aaaaaaaaaaaaaaaaaabp, hash=3f6056bb268f4176685134ae56a838da
n=2, word=aaaaaaaaaaaaaaaaaaqt, hash=311468ef9220551699d61e9b70e43aa7
n=4, word=aaaaaaaaaaaaaaaaafml, hash=314125241241d57b8085afe08b50bdf0
0.10168091144473257


Using a WCF service client and handling its exceptions I am new to WCF and need to work with another programmer's code. I am unsure of the way the WCF service client is used here:

private void barButtonDocuments_ItemClick(object sender, ItemClickEventArgs e)
{
    try
    {
        MyServiceClient myServiceClient = new MyServiceClient();

        try
        {
            documents = myServiceClient.GetDocuments();
            // More code that isn't useful including here ...
        }
        finally
        {
            try
            {
                myServiceClient.Close();
            }
            catch
            {
            }
        }
    }
    catch (FaultException<ServiceErrorDetails> error)
    {
        MessageBox.Show(error.Detail.Message, "Erreur", MessageBoxButtons.OK, MessageBoxIcon.Error);
    }
}

Is it good practice to close the proxy in a finally block, and doing so in an additional try/catch block? I personally don't like the empty catch block because it's like hiding potentially useful exceptions when trying to close the service client.

Are there any better way of handling a WCF service client?

Although paritosh's answer is very helpful, there is additional useful information to take into account when using a service client, as found in this MSDN article :

  It is recommended that a calling application open the channel, use it,
  and close the channel inside one try block.

Let's look at the original try/catch block that was used for the solely purpose of closing the channel :

    try
    {
        myServiceClient.Close();
    }
    catch
    {
    }

This is useless, as stated in the article :

  Datagram channels never fault even if exceptions occur when they are
  closed.

About handling exceptions, MSDN suggests that no unexpected exceptions (like OutOfMemoryException, ArgumentNullException or InvalidOperationException) should be caught when using a service client, as stated in this MSDN article :

  Exceptions that are thrown from communication methods on a Windows
  Communication Foundation (WCF) client are either expected or
  unexpected. Unexpected exceptions include catastrophic failures like
  OutOfMemoryException and programming errors like ArgumentNullException
  or InvalidOperationException. Typically there is no useful way to
  handle unexpected errors, so typically you should not catch them when
  calling a WCF client communication method.

Recommended exceptions to catch while using a service client are TimeoutException and any other exception that derives from CommunicationException.

As the article states :

  One way to handle such errors is to abort the client and report
  the communication failure.

In addition, as this particular project handles it's own FaultException (of type ServiceErrorDetails), the recommended method to catch exceptions while using this service client would be to first catch any TimeoutException, then any FaultException and finally any CommunicationException.

The rewritten code would then look like this :

MyServiceClient myServiceClient = new MyServiceClient();

try
{
    documents = myServiceClient.GetDocuments();
    // More code that isn't useful including here ...
    myServiceClient.Close();
}
catch (TimeoutException exception)
{
    MessageBox.Show(exception.Message, "Timeout error", MessageBoxButtons.OK, MessageBoxIcon.Error);
    myServiceClient.Abort();
}
catch (FaultException<ServiceErrorDetails> error)
{
    MessageBox.Show(error.Detail.Message, "Service error", MessageBoxButtons.OK, MessageBoxIcon.Error);
    myServiceClient.Abort();
}
catch (CommunicationException exception)
{
    MessageBox.Show(exception.Message, "Communication error", MessageBoxButtons.OK, MessageBoxIcon.Error);
    myServiceClient.Abort();
}


A class for form data validation I don't want any feedback on the regexes as I know what needs to be updated here.  Also, don't need any feedback on naming conventions.

I'm looking for feedback on the structure and correctness of the class.

/***************************************************************************************************
**SForm - validates and manipulates form data
*/
    var SForm = $A.Class.create({
        Name: 'SForm',
         S: {
            domain:    /:\/\/(www\.)?([\.a-zA-Z0-9\-]+)/,
            url:       /:\/\/(www\.)?[\x00-\x7F]{1,1800}\.[\x00-\x7F]{1,200}/,
            email:     /\S{1,64}@[\x00-\x7F]{1,255}\.[\x00-\x7F]{1,255}/,
            tweet:     /\S{1,40}/,
            title:     /\S{1,32}/,
            name:      /\S{1,64}/,
            pass:      /\S{6,20}/,
            pre_url:   /(http:)|(https:)\/\//,
            full:      /\S+/,
            google:    'https://plus.google.com/_/favicon?domain='
        },
        constructor : function (form_elements) {
            this.form = {};
            $A.someKey(form_elements, function (val) {
                if (val.type !== 'checkbox') {
                    this.form[val.name] = val.value;
                } else if (val.type === 'checkbox') {
                    this.form[val.name] = val.checked;
                }
            }, this);
        },
        get: function (key) {
            return this.form[key];
        },
        set: function (key, value) {
            this.form[key] = value;
        },
        getObj: function () {
            return this.form;
        },
        checkField: function (key) {
            return this.S[key].test(this.form[key]);
        },
        checkFull: function () {
            var key;
            for (key in this.form) {

                // if it is not a boolean and it is not full
                if (!$A.isBoolean(this.form[key]) && !this.S.full.test(this.form[key]) ) {
                    return false;
                }
            }
            return true;
        },
        checkFullAM: function () {
            var key;
            for (key in this.form) {

                // if the fields is empty and it is not the tag field
                if (key !== "tag" && !this.S.full.test(this.form[key])) {
                    return false;
                }

                // if no tag is set, set one by default
                if (key === "tag" && !this.S.full.test(this.form[key])) {
                    this.form[key] = "no-tag";
                }
            }
            return true;
        },
        addURLPrefix: function () {

            // if there is no prefix, add http by default
            if (!this.S.pre_url.test(this.form.url)) {
                this.form.url = 'http://' + this.form.url;
            }
        },
        setDomain: function () {
            var domain = this.form.url.match(this.S.domain);
            if (domain) {
                this.form.domain = domain[2];
            }
        },
        setFaviconTemp: function () {
            this.form.favicon = this.S.google + this.form.domain;
        }
    }, 'constructor');

A quick review, as the illustrious user34330 is no longer with us.

This:

if (val.type !== 'checkbox') {
    this.form[val.name] = val.value;
} else if (val.type === 'checkbox') {
    this.form[val.name] = val.checked;
}

should be

if (val.type !== 'checkbox') {
    this.form[val.name] = val.value;
} else
    this.form[val.name] = val.checked;
}

because if it is not not a checkbox, then... you know it is checkbox, you could also consider a ternary here:

this.form[val.name] = (val.type === 'checkbox') ? val.checked : val.value;

getObj should be getForm, since it returns the form
checkFullAM the name does not tell me what this does, due to a lack of comments I am mystified at what this function should do. <- That's bad
I would have expected to be able to pass a prefix to addURLPrefix, so that I could set https if I wanted to
setDomain seems to have no possible use, there are no comments to enlighten the reader why/how this could be useful


DO.dom: Creating large DOM structures in Javascript I have the library that I have found to be very useful when creating dynamic content rich web applications. I call it DO.js. It allows me to create entire DOM structures inside of Javascript in a similar manner that HTML markup should look. Additionally, it does it all without writing any HTML strings. It uses a hierarchy of embedded functions and objects to create the DOM structure. Please see my site below for examples.

My question is, is this a good way of doing this? I can and I have (the website below is created in this fashion) created whole websites using this method. I made it so that I could easily load and display large amounts of data pulled via ajax and where each bit of data will have events and variables attached. Of course, this is hard and tacky to do using normal HTML.

I kinda created documentation for it. However, I am not very good with documentation(or writing in general.)

What should/could be done better? What would be the best programming practices in this case?  Anykind of critique would be awesome.

Thanks

http://hurdevan.com/#!Home/Projects/DO.js

Please note that the website does not work at all in IE due to a Hashbang.js bug. And, I don't care about IE.

Code Examples:

Simple Example

with(DO.dom)
   {
   div({parentElement:document.body},
       span(b('Hello World!'))
        );
   }

An example using events and such:

var win = {};
with(DO.dom)
    {
    win.root = div({parentElement:document.body,style:'border:1px solid black;width:200px;height:200px;position:fixed;'},
        win.bar = div({style:'height:20px;text-align:center;background-color:#333333;color:white;cursor:pointer;',
            onmousedown:DO.Func(true,function(eve,winobj)
                {
                window.onmousemove = winobj.bar.onmousemove;
                window.onmouseup = winobj.bar.onmouseup;
                winobj.offsetX = winobj.root.offsetLeft - eve.x;
                winobj.offsetY = winobj.root.offsetTop - eve.y;
                winobj.activeDrag = true;
                },win),
            onmouseup:DO.Func(true,function(eve,winobj)
                {
                window.onmousemove = false;
                window.onmouseup = false;
                winobj.activeDrag = false;
                },win),
            onmousemove:DO.Func(true,function(eve,winobj)
                {
                if(!winobj.activeDrag)return false;
                winobj.root.style.position="fixed";
                winobj.root.style.left = (eve.x + winobj.offsetX)+"px";
                winobj.root.style.top = (eve.y + winobj.offsetY)+"px";
                },win)
            },'Move Window'),

        win.body = div(
            div(b('First Name:'),
                win.firstName = input({type:'text'})
                ),
            div(b('Last Name:'),
                win.lastName = input({type:'text'})
                ),
            div(b('Email:'),
                win.email = input({type:'text'})
                ),
                input({type:'submit',onclick:DO.Func(function(winobj)
                {
                alert("First Name:"+winobj.firstName.value +"\n"+"Last Name:"+winobj.lastName.value +"\n"+"Email:"+winobj.email.value +"\n");
                },win)}),
            "This window was created with DO.dom"
            )
        );

    }

Another example that allows the DOM structure to be child/parent aware.

with(DO.dom)
   {
var foo =div({parentElement:document.body},
       div({nid:'inputElements';},
        input({nid:'firstName'}),
        input({nid:'lastName'}),
        input({nid:'email'})
        )
    );
   }

foo.inputElements.firstName.value = 'Hello';
foo.inputElements.lastName.value = 'Word';
foo.inputElements.email.value = 'hello@world.com';

A more typical approach these days, if you have to create a lot of DOM objects in javascript, is to use a templating system, such as Mustache (see also iCanHazjs), and thus translate javascript objects into HTML.
The advantages are readable javascript and optimised DOM object creation. The disadvantage of this vs. your library is that this doesn't deal with the event binding.

Your library certainly seems powerful, but I would say that this might be very hard to read if you have are creating a lot of DOM objects.
What you'd end up with is a lot of .js code that essentially replicates an HTML document fragment, but with less readability.

This is the advantage of using HTML templates; that you can read the HTML structure easily, but in the javascript code, you don't have more than a line or two of code that does the DOM element creation.  Also, templating systems generally build DOM objects in an efficient manner.

Also, I'd look into something like jQuery entwine as a possibility for easy-to-read event binding.


Find a sum using just two different digits The problem statement was pretty easy and I was able to run half of the test cases, but some of the test cases in the end gave me the error of memory limit reached. What can I do here to improve the memory efficiency of my code?
Question: Find the min number of "usable" integers required to reach the N. The "usable integers" are those that contain only 4 or 5 in their decimal representation, e.g. 4,5,455,545454.
So to reach 9, answer would be 2 (4+5) My approach: Creating a vector consisting of all usable numbers up to N and using dynamic programming solution on that vector to find the minimum number of elements required to reach N.
My code:
int getCount(int N) {
    vector<int> vec;vec.push_back(4);vec.push_back(5);
    int i = 0;int temp = 0;
    while(true) {
        temp = vec[i] * 10 + 4;
        if(temp > N)
            break;
        vec.push_back(temp);
        temp = vec[i] * 10 + 5;
        if(temp > N)
            break;
        vec.push_back(temp);
        i++;
    }

    return getCountDP(vec,vec.size(),N);
}

int getCountDP(vector<int> vec,int size, int N){

    int dp[size+1][N+1];

    for(int i = 0 ; i <= N; i++) {
        dp[0][i] = INT_MAX - 1;
    }

    for(int i = 0 ; i < size+1; i++) {
        dp[i][0] = 0;
    }

    for (int i = 1; i <= size; i++) {
        for (int j = 1; j <= N; j++) {
            if (vec[i - 1] > j) {
                dp[i][j]= dp[i-1][j];
            }
            else {
                dp[i][j]= min(dp[i-1][j],dp[i][j-vec[i-1]]+1);
            }
        }
    }
    return dp[size][N];
}

Since your question is about Memory Limit Exceed. And the most memory consumed item is dp. You are going to make your dp matrix smaller. Your current memory usage is \$O(2^{log_{10}n}\cdot n) = O(n^2)\$. And you need to reduce it to \$O(n)\$.
Let's write down your current dp function:
$$ dp_{i,j} = \min \{ dp_{i-1,j}, dp_{i, j-usable_i}+1 \} $$
First, consider you only need dp[i] and dp[i-1] during iteration. All previous rows are not necessarily any more. You may only declare two rows as dp1[N + 1] and dp2[N + 1]. (To make the logic for even and odd rows consistent, use two pointers point to these array.) By using this approach, memory usage may reduced into \$O(2n) = O(n)\$.
Another approach may be just reduce your dp function into 1 dimension:
$$ dp_{j} = \min_{n \in usable} \{dp_{j-n}\} + 1 $$
So, you may only maintain dp[N + 1] instead. This also cost \$O(n)\$ memory. The second approach also make the array reusable when this function is invoked later. If you are going to handle multiple testcases in one run. You may simply declare it as global variable dp[MAX_N + 1] to reduce its time consume.
#include <stdio.h>

const int MAX_N = 1000000;
const int MAX_VAL = 128; // 2 ^ (log10(MAX_N) + 1)

int getCount(int n) {
  static int filled = 0;
  // values hold all numbers only contais 4 or 5 in 0 ~ 1e6
  static int values[MAX_VAL] = {4, 5};
  // steps is the array for dp
  // steps[i] = min(steps[i - values[j]] + 1 for every j)
  static int steps[MAX_N];
  int i, j, step, min_step;
  if (n > MAX_N) return -1; // We do not support input > 1e6
  // When first time run, we fill all values
  if (filled == 0) {
    for (i = 2, j = 0; i < MAX_VAL; j++) {
      values[i++] = values[j] * 10 + 4;
      values[i++] = values[j] * 10 + 5;
    }
    steps[0] = 0;
    filled = 1;
  }
  for (i = filled > 1 ? filled : 1; i <= n; i++) {
    // We use MAX_N+1 for values cannot be reached
    min_step = MAX_N + 1;
    for (j = 0; values[j] <= i; j++) {
      step = steps[i - values[j]] + 1;
      if (step < min_step) min_step = step;
    }
    steps[i] = min_step;
  }
  filled = n;
  if (steps[n] > n) return -1;
  return steps[n];
}

int main() {
  int n;
  for (n = 0; n < MAX_N; n++) {
    printf("%d\n", getCount(n));
  }
}


Holding server response header data after parsing it I'm creating a program to retrieve web content in order to learn how these things work. I would like it to be as correct as possible, ideally fully compliant.

But I'm not sure this is the right approach to the problem. Is the following structure an appropriate way to hold server response header data after parsing it?

typedef enum {
    OPTIONS = 1, GET, HEAD, POST, PUT, DELETE, TRACE, CONNECT, PATCH
} HTTP_Methods;

typedef enum {
    KEEP_ALIVE = 1, CLOSE
} HTTP_Connection_Field;

typedef enum {
    CHUNKED = 1, COMPRESS, DEFLATE, GZIP, IDENTITY
} HTTP_Transfer_Encoding;

typedef struct {
    uint8_t http_version; //e.g. HTTP/1.1 = 11, HTTP/1.0 = 10, HTTP/0.9 = 9
    uint16_t response_code; //e.g. 200, 404, 500, ...
    char *accept_ranges;
    uint32_t age; //Non-negative, time in seconds, at least 31 bits of range
    HTTP_Methods allow;
    char *cache_control;
    HTTP_Connection_Field connection;
    char *content_encoding;
    char *content_language;
    uint32_t content_length;
    char *content_location;
    char *content_MD5;
    char *content_disposition;
    char *content_range;
    char *content_type;
    time_t date;
    char *etag;
    time_t expires;
    time_t last_modified;
    char *link;
    char *location;
    char *p3p;
    char *pragma;
    char *proxy_authenticate;
    char *refresh;
    time_t retry_after;
    char *server;
    char *set_cookie;
    char *strict_transport_security;
    char *trailer;
    HTTP_Transfer_Encoding transfer_encoding;
    char *vary;
    char *via;
    char *warning;
    char *www_authenticate;
} HTTP_Response_Header;

Maybe grouping the members that are not very commonly used into a different struct that is only allocated when they are in use would be a better idea?

The way I would do it is by simply representing the headers as map of char* to char*. That'll be the most flexible and extensible situation, since HTTP headers are added over time, and custom headers are completely acceptable and allowed for by the standard. Any solution involving struct fields will require updating as the standard expands, and can't handle custom headers easily. (You could handle custom headers the same way as standard ones, but then you'd have the problem that their meaning is not standard, and different clients could use them to represent different things.)

The downside of making the map values be char* is that you'll have to translate into the proper data type wherever you retrieve them from the map, rather than only performing this translation when you read the request headers. This is an acceptable trade-off for the flexibility you gain, and you can find a way to encapsulate it if desired.


Binary Tree for integers This code implements a binary tree for integers. How can I improve it?

#include<stdio.h>
#include<stdlib.h>
#include<iostream>
using namespace std;
///////////////////////////////
struct node{
    int data;
    struct node *left;
    struct node *right;
};
///////////////////////////////
struct node *newnode(int value)
{
    struct node *temp=(struct node *)malloc(sizeof(struct node));
    temp->data=value;
    temp->left=NULL;
    temp->right=NULL;
    return (temp);
}

void inorder_rec(struct node *temp)
{
    if(temp->left!=NULL)
    inorder_rec(temp->left);

    printf("%d",temp->data);

    if(temp->right!=NULL)
    inorder_rec(temp->right);
}

void preorder_rec(struct node *temp)
{
    printf("%d",temp->data);
    if(temp->left!=NULL)
    preorder_rec(temp->left);
    if(temp->right!=NULL)
    preorder_rec(temp->right);
}

void postorder_rec(struct node *temp)
{
    if(temp->left!=NULL)
    postorder_rec(temp->left);
    if(temp->right!=NULL)
    postorder_rec(temp->right);
    printf("%d",temp->data);
}
////////////////////////////////
int main()
{
    int option;
    option=1;
    struct node *root=newnode(1);
    root->left=newnode(2);
    root->right=newnode(3);
    root->left->left=newnode(4);
    root->left->right=newnode(5);
    root->right->left=newnode(6);
    root->right->right=newnode(7);
    printf("Press:\n1 for inorder\n2 for preorder\n3 for postorder\n0         toQUIT!");
    while(option)
    {
        printf("\nenter your option=");
        scanf("%d",&option);
        switch(option)
        {
            case 1:
                inorder_rec(root);
                break;
            case 2:
                preorder_rec(root);
                break;
            case 3:
                postorder_rec(root);
                break;
        /*  case 4:
                inorder_non_rec(root);
                break;
            case 5:
                preorder_non_rec(root);
                break;
            case 6:
                postorder_non_rec(root);
                break;*/

        }

    }
}

C and C++ are different languages.

They have a common syntax in places. But how you use them is completely different. What you have written is C NOT C++.

C++ has its own set of header files for C functionality.

#include<stdio.h>
#include<stdlib.h>

Drop the ".h" and add a "c" prefix for the C++ version of these files.

#include<cstdio>
#include<cstdlib>

This puts all the functions in the namespace std.

Don't use using namespace std;

using namespace std;

Yes we know all tutorials and books do this. But they have a motive they are trying to save space on printed material. In any program larger than 10 lines this is going to cause a problem.

The reason the standard libraries are in the std namespace is so that it is not a huge burden to include the prefix in the name.

More details can be found here: Why is “using namespace std;” considered bad practice? and the best answer.

Use OO and classes

Sure OO is not perfect for everything. But a tree and its node is the perfect place to use OO concepts.

The may it is written is very C like and not C++ like. The constructor is not part of the class. There is no destructor to clean up memory there is no definition of ownership semantics so it is unclear who should be cleaning up memory and thus it is going to leak like a sieve.

struct node{
    int data;
    struct node *left;    // Don't need struct in C++
    struct node *right;   // unlike C structs don't have their own namespace.
};
///////////////////////////////
struct node *newnode(int value)
{
    // Abslutely never use C memory management functions in C++
    // The Heap and Free Store need not map to the same memory.
    // Also this does not invoke the constructure to initialize
    // the object.
    struct node *temp=(struct node *)malloc(sizeof(struct node));

    // Also you C code is broken.
    // malloc() can return `NULL` and you don't check for that.
    temp->data=value;
    temp->left=NULL;
    temp->right=NULL;
    return (temp);
}

This should look like this:

class Tree
{
     // Private class to store the nodes in the tree.
     struct Node
     {
         int   data;
         Node* left;
         Node* right;

         Node(int d) : data(d), left(nullptr), right(nullptr) {}
         Node(int d, Node* l, Node* r) : data(d), left(l), right(r) {}

         ~Node()
         {
              delete left;
              delete right;
         }
     };

     Node*  root;
     Tree()
         : root(nullptr)
     {}
     ~Tree()
     {
         delete root;
     }
     // Remove copy semantics (because I am lazy)
     // If you want add your own
     Tree(Tree const&)            = delete;
     Tree& operator=(Tree const&) = delete;

     void addData(int data) {
        // Ill leave you to implement this
     }
};

These should be a member of the Tree object (Note the Node)

void inorder_rec(struct node *temp)
void preorder_rec(struct node *temp)
void postorder_rec(struct node *temp)

Why are you using printf when you import iostream?

    printf("%d",temp->data);
    printf("%d",temp->data);

Your main just leaks memory. For every call to new there should be a call to delete.  But in reality there should be zero calls to new. That should be done internal to the owning class and most objects should be automatic objects (construct on declaration and destruct when they go out of scope).


Minimum acceptable POM I've been learning how to use Maven, and in order to understand it better, I want to try and write the smallest possible pom.xml file, while being as modern as possible, i.e. using java 1.8, utf-8, having the latest possible version of plugins and dependencies without breaking compatibility.

And so I came up with the following pom.xml and wanted to have other people's opinion on it.

<project>
  <modelVersion>4.0.0</modelVersion>

  <groupId>hello.maven</groupId>
  <artifactId>hello-maven</artifactId>
  <version>0.1.0</version>

  <properties>
    <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
  </properties>

  <build>
    <plugins>
      <plugin>
        <groupId>org.apache.maven.plugins</groupId>
        <artifactId>maven-compiler-plugin</artifactId>
        <version>3.5.1</version>
        <configuration>
          <source>1.8</source>
          <target>1.8</target>
        </configuration>
      </plugin>
    </plugins>
  </build>

  <dependencies>
    <dependency>
      <groupId>junit</groupId>
      <artifactId>junit</artifactId>
      <version>[4,5)</version>
    </dependency>
  </dependencies>
</project>

Edit: Is it possible to have a less redundant version number for maven-compiler-plugin in a way that maven picks the one appropriate to maven's own version?

<version>0.1.0</version>

In Maven, note that the concept of a version that is not released and is still in development is called a snapshot. This means that the version should not be a release version, like 0.1.0, but a snapshot version, like 0.1.0-SNAPSHOT. This is important because it distinguishes clearly what is a version in development and a released version.

As such, consider using a snapshot version here. It is when the Maven project is released, with the help of the maven-release-plugin, that the version will automatically be bumped to a release version (without SNAPSHOT) and tagged on version-control with it.

<plugin>
  <groupId>org.apache.maven.plugins</groupId>
  <artifactId>maven-compiler-plugin</artifactId>
  <version>3.5.1</version>
  <configuration>
    <source>1.8</source>
    <target>1.8</target>
  </configuration>
</plugin>

That is a nice way to configure the Maven project to use Java 8. You can also do it in an alternate way. Both the source and target parameter of the maven-compiler-plugin have a corresponding user property, which are maven.compiler.source and maven.compiler.target, so you can set them in a <properties> section:

<properties>
    <maven.compiler.source>1.8</maven.compiler.source>
    <maven.compiler.target>1.8</maven.compiler.target>
</properties>

This will have the same effect as the current <source> and <target> configuration.

Note that you could not specify a version for the Compiler Plugin in this case; it is the default version coming from the Maven installation that would be used (reference here for 3.3.9). In general, it is a good idea not to rely on this default version and specify explicitly the version to use: this helps in ensuring that the build does not depend on a given Maven version; also it documents clearly the version that is used.

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>[4,5)</version>
</dependency>

Be careful when using version ranges. Ideally, you want to declare dependencies on a single fixed version of a dependency: using a version range is bad practice.

The big advantage of using a fixed version is that it makes it clear what is the expected version to use for your project. What specific version of JUnit do you want your project to depend on? Each version are very different, all of them with their respective features and bugs. With a range, you are basically saying "I don't really care, pick one in major version 4". You could even end up with a SNAPSHOT version (MNG-3092). It should be up to you to pick a version; you can upgrade it later if necessary.

Also, you should use the test scope for the JUnit dependency. It is only needed in the test classpath and is not a compile-time dependency of the main classes.

<dependency>
  <groupId>junit</groupId>
  <artifactId>junit</artifactId>
  <version>4.12</version>
  <scope>test</scope>
</dependency>


Finding the largest mirror image of a subset/set of integers present in an array of integers The problem I am talking about is this:

  We'll say that a "mirror" section in an array is a group of contiguous
  elements such that somewhere in the array, the same group appears in
  reverse order. For example, the largest mirror section in {1, 2, 3, 8,
  9, 3, 2, 1} is length 3 (the {1, 2, 3} part). Return the size of the
  largest mirror section found in the given array.

  maxMirror({1, 2, 3, 8, 9, 3, 2, 1}) → 3
  maxMirror({1, 2, 1, 4}) → 3
  maxMirror({7, 1, 2, 9, 7, 2, 1}) → 2

Conditions for solving:

No other helper methods.
Do not use Java.util.Arrays.copyOf or any other utility under Arrays
Do not use collections.

The solution I got was a little messy, and any cleaner solutions are welcome.

public int maxMirror(int[] nums) {
  final int len=nums.length;
  if(len==0)
  return 0;
  int maxCount=1;
  boolean flag=false;

  for(int i=0;i<len;i++)
  {
     int tempCount=1;
     int count=i;
     flag=false;

     for(int j=len-1;j>=0&&(count<len);j--)
     {
        if((nums[count]==nums[j])&&!(flag))
        {
          flag=true;
          count++;
          continue;
        }
        if((nums[count]==nums[j])&&(flag))
        {
          tempCount++;
          count++;
          maxCount=(tempCount>maxCount)?tempCount:maxCount;
         continue;
        }
        if(!(nums[i]==nums[j])&&(flag))
        {
          flag=false;
          count=i;
          tempCount=1;
          continue;
        }
        if((j==count)||(j-count)==1)
        {
          flag=false;
          break;
          }

      }
  }
      return maxCount;

}

Readibility

Whitespaces are free and make things easier to read. Also, you do not need that many parenthesis.

Also, indentation seems a bit weird. After fixing this, here is what I have :

public class MaxMirror {

    public static int maxMirror(int[] nums) {
        final int len = nums.length;
        if (len == 0)
            return 0;
        int maxCount = 1;
        boolean flag = false;

        for (int i = 0; i<len; i++)
        {
            int tempCount = 1;
            int count = i;

            for (int j = len-1; j>= 0 && (count<len); j--)
            {
                if (nums[count] == nums[j] && !flag)
                {
                    flag = true;
                    count++;
                    continue;
                }
                if (nums[count] == nums[j] && flag)
                {
                    tempCount++;
                    count++;
                    maxCount = (tempCount>maxCount)?tempCount:maxCount;
                    continue;
                }
                if (nums[i] != nums[j] && flag)
                {
                    flag = false;
                    count = i;
                    tempCount = 1;
                    continue;
                }
                if (j == count || (j-count)==1)
                {
                    flag = false;
                    break;
                }

            }
        }
        return maxCount;
    }

    public static void main(String[] args) {
        System.out.println("Hello, world!");
        int[] num = {1, 2, 3, 8, 9, 3, 2, 1};
        System.out.println(maxMirror(num));
        int[] num2 = {1, 2, 1, 4};
        System.out.println(maxMirror(num2));
        int[] num3 = {7, 1, 2, 9, 7, 2, 1};
        System.out.println(maxMirror(num3));
    }
}

Re-writting the logic

Instead of using continue, you could just use else between your conditions.

Also, if you have to consider A && B and then A && !B, you should probably consider A and then, as a subcase, the validity of B.

Then, you can remove common code from the then block and the else block.

You can use max instead of checking which value is bigger.

You could move your check count < len to the only place where count could become bigger than len.

You can rewrite (j-count)==1 to make it look like the previous expression : j == (count+1) seems slightly better.

This being done, your code looks like :

public static int maxMirror(int[] nums) {
    final int len = nums.length;
    if (len == 0)
        return 0;
    int maxCount = 1;
    boolean flag = false;

    for (int i = 0; i<len; i++)
    {
        int tempCount = 1;
        int count = i;

        for (int j = len-1; j>= 0; j--)
        {
            if (nums[count] == nums[j])
            {
                if (flag)
                {
                    tempCount++;
                    maxCount = Math.max(tempCount, maxCount);
                }
                flag = true;
                count++;
                if (count>=len)
                    break;
            }
            else if (nums[i] != nums[j] && flag)
            {
                flag = false;
                count = i;
                tempCount = 1;
            }
            else if (j == count || j == (count+1))
            {
                flag = false;
                break;
            }
        }
    }
    return maxCount;
}

Algorithm

Your algorithm seems to be working. However, I find it hard to understand. I guess a bit of documentation would be useful.

Edit : bug found ?

On int[] num5 = {7, 7, 7, 7, 6, 7, 7};, the function returns 6 and I do not see why.

Edit 2

I originally posted this as a different answer because it is not related to the beginning of this message in any way but this doesn't seem to be much appreciated. As it can be relevant to you, I'm posting this here.

As your code doesn't really work and I got quite interested by this problem : I took mjolka's precious comments into account.

Re-adapting code from the link in his comment, here what I got :

public class MirrorString {
    /* Returns length of longest common substring of X and Y */
    public static int LCSubStr(int[] X /* WAS , int[] Y*/)
    {
        int m = X.length;
        int n = m; // WAS int n = Y.length;

        // Create a table to store lengths of longest common suffixes of
        // substrings.   Notethat LCSuff[i][j] contains length of longest
        // common suffix of X and Y. The first row and
        // first column entries have no logical meaning, they are used only
        // for simplicity of program
        int[][] LCSuff = new int[m+1][n+1];
        int result = 0;  // To store length of the longest common substring

        /* Following steps build LCSuff[m+1][n+1] in bottom up fashion. */
        for (int i=0; i<=m; i++)
        {
            for (int j=0; j<=n; j++)
            {
                if (i == 0 || j == 0)
                    LCSuff[i][j] = 0;
                else if (X[i-1] == X[n-j]) // WAS else if (X[i-1] == Y[j-1])
                {
                    LCSuff[i][j] = LCSuff[i-1][j-1] + 1;
                    result = Math.max(result, LCSuff[i][j]);
                }
                else LCSuff[i][j] = 0;
            }
        }
        return result;
    }

    public static void main(String[] args) {
        System.out.println("Hello, world!");
        System.out.println(LCSubStr(new int[] {7, 7, 7, 5, 6, 7, 7})); // 3
        System.out.println(LCSubStr(new int[] {7, 7, 7, 7, 6, 7, 7})); // 5
        System.out.println(LCSubStr(new int[] {})); // 0
        System.out.println(LCSubStr(new int[] {1})); // 1
        System.out.println(LCSubStr(new int[] {1, 1})); // 2
        System.out.println(LCSubStr(new int[] {1, 1, 1})); // 3
        System.out.println(LCSubStr(new int[] {1, 2, 3, 2, 1})); // 5
        System.out.println(LCSubStr(new int[] {1, 2, 3, 8, 9, 3, 2, 1})); // 3
        System.out.println(LCSubStr(new int[] {1, 2, 1, 4})); // 3
        System.out.println(LCSubStr(new int[] {7, 1, 2, 9, 7, 2, 1})); // 2
    }
}

I have adapted the code a bit. Then, the only places where the algorithm has been changed are marked with WAS :

only one argument is required now
instead of accessing the j-1th element from J, we access the n-jth element from X (simulating a backward traversal).

Please note that things could be done in an even more efficient way using a different data structure as per the wikipedia page about Longest Common Substring problem.


DSL for describing circuits I am working on a micro project: a Haskell DSL for describing circuits.
(You can find the code here)

I'm pretty happy with everything so far, but there is an ugly function alu32:

    alu32 c0 c1 c2 a b =
      let aluc = alu1 c0 c1 c2
          (out0, cout0) = aluc (a !! 0) (b !! 0) c0
          (out1, cout1) = aluc (a !! 1) (b !! 1) cout0
          (out2, cout2) = aluc (a !! 2) (b !! 2) cout1
          (out3, cout3) = aluc (a !! 3) (b !! 3) cout2
                     ...

Can anyone suggest a more idiomatic way of representing this? Also, if you feel like browsing the repository, feel free to make any suggestions. I have a bunch of things that I would still like to implement.

Can anyone suggest a more idiomatic way of representing this?

The equations for the ALU have the form

(out !! i, cout !! i) = aluc (a !! i) (b !! i) (cout !! (i - 1))

so we should be able to express all of the equations with a single equation that manipulates lists. If only cout !! (i - 1) would be something !! i, this would be easier because all the indexes would be uniform. We can make the indexes more uniform by defining cin = c0 : cout, and then the equations become:

(out !! i, cout !! i) = aluc (a !! i) (b !! i) (cin !! i)

Now we can make a loop over i to implement alu32 as follows:

alu32 c0 c1 c2 a b =
  let (out, cout) = unzip [alu1 c0 c1 c2 (a !! i) (b !! i) (cin !! i) | i <- [0 .. 31]]
      cin = c0 : cout
      ...
  in ...

This should solve the main question: How to implement alu32 without repeating all these equations. On the way, I also replaced the translate with an unzip.

However, we can do even better if we avoid the index computations. There are two problems with the indices: Index lookup is slow and awkward compared to just looping over the elements of a list. And the loop over the indices forces us to choose a fixed upper bound in the [0 .. 31] expression.

To avoid the indices, we note that the list comprehension

[alu1 c0 c1 c2 (a !! i) (b !! i) (cin !! i) | i <- [0 .. 31]]

just loops over three lists in parallel, a, b and cin. This parallel loop over three lists can be expressed with zipWith3 as follows:

unzip (zipWith3 (alu1 c0 c1 c2) a b cin)

Or you activate GHC's extension for parallel list comprehensions by putting {-# LANGUAGE ParallelListComp #-} near the top of the file, and then use:

unzip [alu1 c0 c1 c2 ai bi ci | ai <- a | bi <- b | ci <- cin]

The latter will desugar to the former. Clearly, this variant of the code doesn't use any indices anymore. This also means that it works for arbitrary bitvectors, not just 32bit ones. We only have to adapt the rest of the alu32 implementation to avoid indices, too. For example:

alu :: Wire -- Control Bit 0
  -> Wire -- Control Bit 1
  -> Wire -- Control Bit 2
  -> ByteWire -- n Bit Wire addend
  -> ByteWire -- n Bit Wire addend
  -> (ByteWire, Wire, Wire, Wire)
alu c0 c1 c2 a b =
  let (out, cout) = unzip (zipWith3 (alu1 c0 c1 c2) a b cin)
      cin = c0 : cout
      overflow = xorGate (take 2 (reverse cout))
      isZero   = map (all (==False)) out
      negative = last out
  in (out, overflow, isZero, negative)

This version of alu is concise and works for bitvectors of arbitrary length.

I'm not really sure about map (all (== False)) out to compute that all bits are zero. Maybe I would use something like map (all not) out or map (not . or) out. Note sure. Or maybe I would define a gate for deciding that something is all zeros and then use that one.

  Also, if you feel like browsing the repository, feel free to make any suggestions.

Consider adding a cabal file. When I cloned your repo in order to test the above suggestion, I was confused that cabal sandbox init && cabal build didn't work. You can just run cabal init in the repository and it should ask you questions to help you make the cabal file.
Consider making the Wire type more abstract.

Currently, you have:

type Wire = [Binary] -- A list of discrete states

Instead, you could use:

newtype Wire = Secret [Binary]

The benefit of this definition is that it becomes harder to "cheat" and use list operations instead of gates. Later, when you are finished defining the basic gates, you can make a module that exports the Wire type and the gates, but not the Secret constructors. This way, you force the users of that module to use the gates and only the gates to compose wires.


ShuffleBag (Fisher-Yates + PcgRandom) Anything that could be done better here?

public class ShuffleBag<T> : IEnumerable<T>
{
    private readonly int m_endIndex;
    private readonly FastRandom m_rng;
    private readonly T[] m_values;

    private int m_currentIndex;

    public ShuffleBag(T[] values, ulong state, ulong? stream = null) {
        m_currentIndex = 0;
        m_endIndex = (values.Length - 1);
        m_rng = (stream.HasValue ? new FastRandom(state, stream.Value) : new FastRandom(state));
        m_values = values;
    }
    public ShuffleBag(T[] values) : this(values, SecureRandom.GetUInt64()) { }

    IEnumerator IEnumerable.GetEnumerator() {
        return GetEnumerator();
    }
    public IEnumerator<T> GetEnumerator() {
        var currentIndex = m_currentIndex;
        var endIndex = m_endIndex;
        var rng = m_rng;
        var values = m_values;

        while (currentIndex <= endIndex) {
            var randomIndex = rng.NextInt32(currentIndex, endIndex);
            var temp = values[randomIndex];

            values[randomIndex] = values[currentIndex];
            values[currentIndex] = temp;

            m_currentIndex = currentIndex++;

            yield return temp;
        }
    }
}

For those that might inquire, here's the source for FastRandom and SecureRandom.

Enumerate once

I'm not sure whether you intended to write this to be iterable only once. Is this as designed?

[TestMethod]
public void CodeReview()
{
    var bag = new ShuffleBag<string>(new[] { "a", "b", "c", "d" });

    var e1 = bag.ToList();  // randomized: "b", "a", "c", "d"
    var e2 = bag.ToList();  // last element: "d"
    var e3 = bag.ToList();  // last element: "d"
}

Collections interfaces

I would let the bag inherit IReadOnlyCollection<T> to provide eager access to the bag's size:

public int Count => m_values.Length;


Subtable timestamps in python pandas I have a dataframe like:

    time_stamp           id          next_timestamp
0   2010-04-16 11:57:52  string_1    NaT
1   2010-04-16 12:06:16  string_1    NaT
2   2010-04-16 12:40:53  string_2    NaT

I want to fill next_timestamp column, with the next time_stamp that id has (if it exists).

Result would be something like:

    time_stamp           id          next_timestamp
0   2010-04-16 11:57:52  string_1    2010-04-16 12:06:16
1   2010-04-16 12:06:16  string_1    NaT
2   2010-04-16 12:40:53  string_2    NaT

My code at the moment:

for row in df.index:
    row_time_stamp = df.time_stamp[row]
    id_array = df.id[row]
    df_temp = df.loc[(df['time_stamp'] >= row_time_stamp) & \
                     (df['time_stamp'] <= row_time_stamp + datetime.timedelta(days=7))]
    try:
        next_id_msg = df_temp.loc[(df_temp['id'] == str(id_array))].time_stamp.min()
        df['next_timestamp'][row] = next_id_msg
    except IndexError:
          df['next_timestamp'][row] = pd.NaT

The problem is that my my df is 50+ million rows long, and setting up a temp table for every row is not a good pattern.

Please help me out with a better pattern.

you can use groupby.transform

df["next_timestamp"] = df.groupby("id")["time_stamp"].transform(
    lambda x: x.shift(-1)
)


Custom algorithm for hashing and un-hashing password I am attempting to create functions for getting an algorithm, hashing and un-hashing a string.

As it stands, it works, and very well. But, I feel this is not secure enough and someone could easily break it!

Can someone help me to improve this?

Code:

<?php
function HashIt($string)
{
    $return = '';
    $algo = GetAlgo();
    $hashing = str_split($string, 1);
    foreach ($hashing as $part)
    {
        $return .= $algo[$part] . "|";
    }
    return rtrim($return, "|");
}

function UnhashIt($hash)
{
    $return = '';
    $algo = GetAlgo();
    $unhash = explode("|", $hash);
    foreach ($unhash as $value)
    {
        $find = array_search($value, $algo);
        if ($find)
            $return .= $find;
    }
    return $return;
}

function GetAlgo()
{
    return [
        "a"=>"[H]",
        "b"=>"[He]",
        "c"=>"[Li]",
        "d"=>"[Be]",
        "e"=>"[B]",
        "f"=>"[C]",
        "g"=>"[N]",
        "h"=>"[O]",
        "i"=>"[F]",
        "j"=>"[Ne]",
        "k"=>"[Na]",
        "l"=>"[Mg]",
        "m"=>"[Al]",
        "n"=>"[Si]",
        "o"=>"[P]",
        "p"=>"[S]",
        "q"=>"[Cl]",
        "r"=>"[Ar]",
        "s"=>"[K]",
        "t"=>"[Ca]",
        "u"=>"[Sc]",
        "v"=>"[Ti]",
        "w"=>"[V]",
        "x"=>"[Cr]",
        "y"=>"[Mn]",
        "z"=>"[Fe]",
        "A"=>"[Co]",
        "B"=>"[Ni]",
        "C"=>"[Cu]",
        "D"=>"[Zn]",
        "E"=>"[Ga]",
        "F"=>"[Ge]",
        "G"=>"[As]",
        "H"=>"[Se]",
        "I"=>"[Br]",
        "J"=>"[Kr]",
        "K"=>"[Rb]",
        "L"=>"[Sr]",
        "M"=>"[Y]",
        "N"=>"[Zr]",
        "O"=>"[Nb]",
        "P"=>"[Mo]",
        "Q"=>"[Tc]",
        "R"=>"[Ru]",
        "S"=>"[Rh]",
        "T"=>"[Pd]",
        "U"=>"[Ag]",
        "V"=>"[Cd]",
        "W"=>"[In]",
        "X"=>"[Sn]",
        "Y"=>"[Sb]",
        "Z"=>"[Te]",
        " "=>"[I]",
        "1"=>"[Xe]",
        "2"=>"[Cs]",
        "3"=>"[Ba]",
        "4"=>"[Hf]",
        "5"=>"[Ta]",
        "6"=>"[W]",
        "7"=>"[Re]",
        "8"=>"[Os]",
        "9"=>"[Ir]",
        "0"=>"[Pt]",
        "!"=>"[Au]",
        "£"=>"[Hg]",
        "$"=>"[Tl]",
        "%"=>"[Pb]",
        "^"=>"[Bi]",
        "&"=>"[Po]",
        "*"=>"[At]",
        "("=>"[Rn]",
        ")"=>"[Fr]",
        "_"=>"[Ra]",
        "-"=>"[Rf]",
        "="=>"[Db]",
        "+"=>"[Sg]",
        "`"=>"[Bh]",
        "¬"=>"[Hs]",
        ""=>"[Mt]",
        ","=>"[Ds]",
        "<"=>"[Rg]",
        "."=>"[Cn]",
        ">"=>"[Uut]",
        "/"=>"[Fl]",
        "?"=>"[Uup]",
        ";"=>"[Lv]",
        ":"=>"[Uus]",
        "'"=>"[Uuo]",
        "@"=>"[La]",
        "#"=>"[Ce]",
        "~"=>"[Pr]",
        "["=>"[Nd]",
        "]"=>"[Pm]",
        "{"=>"[Sm]",
        "}"=>"[Eu]",
        "."=>"Gd",
    ];
}
?>

First of all, what you have is an encryption function, not a hashing function (encryption can be reversed by knowing the algorithm and optionally a password, hashing is one way).

For passwords, you always* want to hash, not encrypt (because if an attacker gets your source code, they can easily decrypt the passwords).

You are using a substitution cipher, which is also very easy to break. You might want to look into block or stream ciphers, if you want encryption that is more secure.

As to the code you do have:

function names should start with a lower-case character (classes start with an uppercase one).
with the information given above, it's obvious that the function names don't really fit. Also, you don't need filler words such as It in function names. Something like encrypt(plaintext) and decrypt(ciphertext) would be better.
You shouldn't silently ignore errors. I would add an else clause to your if ($find) and throw an exception.
In UnhashIt you check if the string exists, but you don't do it for the more important HashIt function. This means that UnhashIt(HashIt(text)) != text in some cases, which is not good.
For any real-world code, use bcrypt.

* there are exceptions such as kerberos and password managers, but in use cases where you want to authenticate a user by checking a user supplied password against a stored password (ie whenever you do have a user supplied plaintext password to check against), hashing is the correct way, not encryption
(see the comments/chat for an extended discussion)


Getting the dominant RGB color of a bitmap I'm currently running this function 60 times per second in order to get the dominant color on my screen in RGB values. It is using approximately 15% of my CPU on 30FPS and 25% of my CPU on 60FPS. Is there any way I can improve the efficiency of this loop or is there a better way to get the color altogether?

public Color getDominantColor(System.Drawing.Bitmap bmp) {
            BitmapData srcData = bmp.LockBits(new Rectangle(0, 0, bmp.Width, bmp.Height), ImageLockMode.ReadOnly, System.Drawing.Imaging.PixelFormat.Format32bppArgb);

            int stride = srcData.Stride;

            IntPtr Scan0 = srcData.Scan0;

            int[] totals = new int[] { 0, 0, 0 };

            int width = bmp.Width;
            int height = bmp.Height;

            unsafe
            {
                byte* p = (byte*)(void*)Scan0;

                for (int y = 0; y < height; y++) {
                    for (int x = 0; x < width; x++) {
                        for (int color = 0; color < 3; color++) {
                            int idx = (y * stride) + x * 4 + color;
                            totals[color] += p[idx];
                        }
                    }
                }
            }

            int avgB = totals[0] / (width * height);
            int avgG = totals[1] / (width * height);
            int avgR = totals[2] / (width * height);

            bmp.UnlockBits(srcData);

            return Color.FromArgb(avgR, avgG, avgB);
        }

If we look at your for loop very carefully we can see that you can eliminate two multiplications:

for (int y = 0; y < height; y++) {
    for (int x = 0; x < width; x++) {
        for (int color = 0; color < 3; color++) {
            int idx = (y * stride) + x * 4 + color;
            totals[color] += p[idx];
        }
    }
}

You don't use y except for y * stride, and you don't use x except for x * 4, rewrite the for loop and you can eliminate those entirely.

var heightLimit = height * stride;
var widthLimit = width * 4;

for (int y = 0; y < heightLimit; y += stride) {
    for (int x = 0; x < widthLimit; x += 4) {
        for (int color = 0; color < 3; color++) {
            int idx = y + x + color;
            totals[color] += p[idx];
        }
    }
}

By removing all three multiplication operations, we effectively reduce the amount of work we do by a significant portion. (There are roughly 8 instructions in the original, and 6 in the reduced, so we've eliminated 25% of our work.)

Other than that, there really isn't much you can do. You could consider chunking the work, then only recalculate the regions that were re-drawn if that's reasonable, you could also chunk and thread it (this wouldn't reduce CPU usage but it would reduce time spend on this method).

With as frequently as you call this method, the next lines may be a potential for optimization as well:

int avgB = totals[0] / (width * height);
int avgG = totals[1] / (width * height);
int avgR = totals[2] / (width * height);

Why do width * height in all three calculations? Why not store var pixelCount = width * height; then divide by pixelCount? Of course, division is still slow, but you're not using floating-point math so we can't use the reciprocal of it.

You could consider, as mentioned in a comment, using CUDA/OpenGL/GPU-level work. Basically, operating on the GPU itself instead of using the CPU to do what could be very efficient on the GPU. (It's built specifically for this type of processing.) There is at least one Stack Overflow question on running C# code on the GPU, it's not very easy or simple, but it can give you a lot of power.


malloc implementation Please give me some feedback to my malloc implementation.  I implemented this design to understand the basic concept.

#include <stdio.h>
#include <stdbool.h>
#include <string.h>

#define HEAP_MEM_SIZE 150
#define MEMORY_AVAILABLE 1
#define MEMORY_USED 0

struct chunk_header {
  struct chunk_header *next; // next pointer on free list
  size_t size;               // the size of this chunk
  bool is_available;         // indicates if this chunk is MEMORY_AVAILABLE or MEMORY_USED
};

typedef struct chunk_header chunk_header;
chunk_header *chunk_header_begin;
static char buffer[HEAP_MEM_SIZE];
unsigned int heap_size;

void init() {
    heap_size = HEAP_MEM_SIZE;
    chunk_header_begin = (chunk_header*)&buffer;
    chunk_header_begin->next = NULL;
    chunk_header_begin->size = heap_size;
    chunk_header_begin->is_available = MEMORY_AVAILABLE;
}

void init_next_chunk(chunk_header *curr, unsigned int bytes) {
    heap_size -= bytes;
    curr->next = NULL;
    curr->size = heap_size;
    curr->is_available = MEMORY_AVAILABLE;
}

void *my_malloc(unsigned int nbytes) {
    static bool init_flag = false;

    if(!init_flag) {
        init();
        init_flag = true;
    }

    int alloc_size = nbytes + sizeof(chunk_header);
    chunk_header *curr = chunk_header_begin;

    while(curr) {
        if(curr->is_available && curr->size >= alloc_size) {
            curr->is_available = MEMORY_USED;
            curr->size = alloc_size;
            curr->next = curr + alloc_size;

            init_next_chunk(curr->next, alloc_size);

            // return memory region
            curr = curr + sizeof(chunk_header);
            return curr;
        }

        curr = curr->next;
    }

    return NULL;
}

void my_free(void *firstbyte) {
    chunk_header *mem = (chunk_header*)firstbyte - sizeof(chunk_header);
    chunk_header *curr = chunk_header_begin;

    while (curr) {
        if (curr == mem) {
            heap_size += curr->size;

            // Mark the block as being available
            curr->is_available = MEMORY_AVAILABLE;
            break;
        }

        curr = curr->next;
    }

    firstbyte = NULL;
}

void print_heap_allocations() {
    chunk_header *curr = chunk_header_begin;

    printf("\n\tSize\tAvailable\tMemory-Ptr");

    while (curr) {
        void *mem_ptr = curr + sizeof(chunk_header);
        printf("\n\t%d\t%d\t\t%x", curr->size, curr->is_available, mem_ptr);
        curr = curr->next;
    }

    printf("\n");
}

int main() {
    char *mem1 = (char*)my_malloc(20);
    if(mem1 == NULL) {
        goto err;
    }
        memset (mem1,'x',19);
        mem1[19] = '\0';

        print_heap_allocations();

        char *mem2 = (char*)my_malloc(20);
        if(mem2 == NULL) {
        goto err;
    }
        memset (mem2,'y',19);
        mem2[19] = '\0';

        print_heap_allocations();

        char *mem3 = (char*)my_malloc(20);
        if(mem3 == NULL) {
            goto err;
        }
        memset (mem3,'z',19);
        mem3[19] = '\0';

        print_heap_allocations();

    my_free(mem2);

    print_heap_allocations();

    char *mem4 = (char*)my_malloc(20);
    if(mem4 == NULL) {
        goto err;
    }
        memset (mem4,'a',20);
    mem4[19] = '\0';

        print_heap_allocations();

        printf("should be 19 x sein: %s\n", mem1);
        printf("should be 19 a sein: %s\n", mem4);
        printf("should be 19 z sein: %s\n", mem3);

    return 0;

err:
        printf("could not allocate mem\n");
        return 0;
}

Things you did well:

Some use of comments.
Use of typedef.

Things you could improve:

Syntax/Styling:

Never use goto.

Yes, there are some rare situations where you may find it necessary to use it.  This is not one of them.

err:
        printf("could not allocate mem\n");
        return 0;

As you can see, all you do in your little block of code here is print out an unspecific error message,  and then exit (using 0, which means the program was successful and is contradictory here).

You should instead abolish the goto's completely.  Print out specific, useful error messages instead, and then return with a unique exit code.

if(mem2 == NULL)
{
    fprintf(stderr, "Error allocating memory to mem2.\n");
    return -2;
}

Your format specifies type int when you are trying to print a string, but the argument has type size_t (basically, unsigned long).  Your format also specifies type unsigned int but the argument has type void *.

printf("\n\t%d\t%d\t\t%x", curr->size, curr->is_available, mem_ptr);

Just change they type you are trying to print out, and cast mem_ptr to an unsigned int.

printf("\n\t%zuu\t%d\t\t%x", curr->size, curr->is_available, (unsigned int) mem_ptr);

You aren't using standard naming conventions with your typedef struct.

typedef struct chunk_header chunk_header;

You should be giving the typedef a specific name, different from the abstract struct name.
You should be capitalizing the name of the type, or adding a _t to the end (though this is reserved in POSIX)

typedef your structs when you declare them.

struct chunk_header {
 struct chunk_header *next; // next pointer on free list
 size_t size;               // the size of this chunk
 bool is_available;         // indicates if this chunk is MEMORY_AVAILABLE or MEMORY_USED
};
typedef struct chunk_header chunk_header;

The typedef means you no longer have to write struct all over the place, as you have used it.  But you can combine these two parts to make things more simple:

typedef struct chunk_header
{
    struct chunk_header *next; // next pointer on free list
    size_t size;               // the size of this chunk
    bool is_available;         // indicates if this chunk is MEMORY_AVAILABLE or MEMORY_USED
} ChunkHeader;

You can simplify your NULL checks.

 if(mem1 == NULL)

Since NULL is defined as (void *)0, we can treat is as a comparison to 0.

if(!mem1)

Parameters/Returns:

You don't accept any parameters for some of your functions, such as main().  If you do not take in any parameters, declare them as void.

int main(void)

You should still return from a function, even if it is declared to return void.

void my_free(void *firstbyte) {
    ...
    return;
}

Compilation:

I can see from some warnings I received that you have not enabled all of your compiler warnings, which you should do.


C++ Template to implement the Factory Pattern Design

A template class that will create a concrete instance of an interface class when given the name (as a string) of the concrete class.

This design is often seen in code, so the purpose of this class is to reduce the boilerplate code needed to write a factory class.

The template class will be used like this:

using MyFactory = factory<MyInterfaceType, MyType_1, MyType_2, MyType_3>;

int main() {
    auto ptr = MyFactory::create("MyType_1");
    ptr->doSomething();
}

There will be a static assert to check that all of the concrete types inherit from the interface type and the create method can accept an optional argument to describe the return type if no class matching the id is found.

Implementation

template<typename interface_type, typename... products>
struct factory {

    template<typename Class>
    struct LabledClass {
        std::string_view label = to_string<Class>();
        Class data;
    };

    using labled_classes = std::tuple<LabledClass<products>...>;

    static std::unique_ptr<interface_type> create(const std::string_view& id, std::unique_ptr<interface_type> default_type = nullptr) {
        std::unique_ptr<interface_type> result = std::move(default_type);

        //checl all products inherit from interface_type
        std::apply([](auto&&... tuple_elem) {
            ((static_check<decltype(tuple_elem.data)>()), ...);
        }, labled_classes{} );

        //if product matches id, return a new instance of that product
        std::apply([&result, &id](auto&&... tuple_elem) {
            (( tuple_elem.label == id ? result = std::make_unique<decltype(tuple_elem.data)>() : result ), ...);
        }, labled_classes{} );

        return result;
    }

private:
    template<typename product>
    static void static_check() {
        static_assert(std::is_base_of<interface_type, product>::value, "all products must inherit from interface_type");
    }
};

This requires the function to_string<Class>() which does not yet exist in the c++ standard, but can be implemented on GCC or Clang like so:

template<typename Class>
constexpr std::string_view to_string() {
    std::string_view str = __PRETTY_FUNCTION__;
    auto first = str.find("= ");
    std::string_view str2 = str.substr(first + 2);
    auto last = str2.find(";");
    str2.remove_suffix(str2.length() - last);
    return str2;
}

Example use

#include <iostream>
#include "factory.h"

struct Animal {
    virtual void makeNoise() const = 0;
};

struct Dog : Animal {
    virtual void makeNoise() const override{
        std::cout << "Woof" << std::endl;
    }
};

struct Cat : Animal {
    virtual void makeNoise() const override{
        std::cout << "Meow" << std::endl;
    }
};

struct Duck : Animal {
    virtual void makeNoise() const override{
        std::cout << "Quack" << std::endl;
    }
};

struct NullAnimal : Animal {
    virtual void makeNoise() const override{
        std::cout << "?" << std::endl;
    }
};

using AnimalFactory = factory<Animal, Dog, Cat, Duck>;

int main() {
    auto animal = AnimalFactory::create("Dog");
    animal->makeNoise();
}

I have tested this with GCC 9.01 and it works

Seems pretty reasonable. I mean, I definitely wouldn't put this in production code because it relies on parsing a class name out of __PRETTY_FUNCTION__, and that's not necessarily guaranteed to keep working in future versions of GCC let alone Clang (and __PRETTY_FUNCTION__ isn't even supported at all on MSVC; they have __FUNC_SIG instead).

In fact, I just tested on Godbolt, and your to_string<T>() function doesn't work at all on Clang. Furthermore, even on GCC, it has trouble with corner cases like A<';'> — https://godbolt.org/z/UNVRQL

Nitpicks on your test code: The rule of thumb I follow is that every polymorphic method should have exactly one of virtual, override, or final (and really nothing should ever have final). So your repetition of virtual is just clutter, to me.

Ditto your use of std::endl (which flushes) versus plain old "\n" (which also flushes if you're outputting to a line-buffered stream such as std::cout). You could save some typing there.

You misspell "labeled" in at least two places: LabledClass and labled_classes. These are implementation details, but it's still important to spell things right so that you can grep for them later.

You pass const std::string_view& id by reference. This is unidiomatic. string_view is already a trivially copyable type, the size of two pointers. It doesn't make sense to force one of those onto the stack just so you can take its address and pass it by reference. Pass string_view by value.

    //checl all products inherit from interface_type
    std::apply([](auto&&... tuple_elem) {
        ((static_check<decltype(tuple_elem.data)>()), ...);
    }, labled_classes{} );

Typo: checl for check. And this is waaay more complicated than it needs to be. Just static_assert the thing you want to assert:

static_assert(std::is_base_of_v<interface_type, products> && ...);

In fact, let's use the idiomatic CamelCase for template arguments, and keep them short:

template<class Base, class... Ps>
[...]
    static_assert(std::is_base_of_v<Base, Ps> && ...);

std::apply([&result, &id](auto&&... tuple_elem) {
    (( tuple_elem.label == id ?
       result = std::make_unique<decltype(tuple_elem.data)>() :
       result ), ...);
}, labled_classes{} );

This complexity is a little more irreducible, but still, doing it all with a tuple and std::apply seems like way more template instantiations than you really ought to have here. What's wrong with a good old-fashioned chain of ifs?

Also, nit: if you're capturing everything by reference, just write [&]. It saves brain cells for the reader of your code.

int dummy[] = {
    ([&]() { if (id == to_string<Ps>()) result = std::make_unique<Ps>(); }(), 0) ...
};

We could even short-circuit as soon as we find the match. That's easy if we leave result null at first; then result will be null if and only if we should still be doing string comparisons.

static std::unique_ptr<Base>
create(std::string_view id, std::unique_ptr<Base> default_type = nullptr)
{
    static_assert(std::is_base_of_v<Base, Ps> && ...);

    std::unique_ptr<Base> result = nullptr;
    int dummy[] = {
        ([&]() {
            if (result == nullptr && id == to_string<Ps>()) {
                result = std::make_unique<Ps>();
            }
        }(), 0) ...
    };
    if (result == nullptr) {
        result = std::move(default_type);
    }
    return result;
}

At this point it's no longer clear why you need struct factory at all. So personally I'd get rid of it, and rename the now-free function create to makeUniqueByName or something.


Creating a singleton container class for intrer-class messaging My question is for validation of my code (any loop holes / bugs) and guidance the best  methodology to implement for my requirement.

I am developing a Python application that will have many classes. We need to communicate in between each class - pass data. It would be very clumsy if across all classes I create a setter/getter method and refer them across different classes, hence I prefer to pass a container object in between all of them:

class One
  def __init__(self, container):

class Two
  def __init__(self, container):

Now obviously I want that this container object should be a Singleton object - only one instance to pass multiple data structures. Below is my code for same [class declaration for Container class]:

import abc

class Container(object):
    __metaclass__ = abc.ABCMeta
    __data1 = 0
    __data2 = 0
    @abc.abstractmethod
    def dummy():
        raise NotImplementedError()
    def __init__(self):
        self.__data1 = 0
        self.__data2 = 1
    def SetData(self,value):
        self.__data1 = value
    def GetData(self):
        return self.__data1

class Singleton(Container):
    _instances = {}
    def __new__(class_, *args, **kwargs):
        if class_ not in class_._instances:
                class_._instances[class_] = super(Singleton, class_).__new__(class_, *args, **kwargs)
        return class_._instances[class_]
    def dummy():
        pass

Now in my application, I would not be able to create an another separate instance of Container [abstract class] / Singleton class. So I can pass the object of Singleton like as mentioned below:

class One:
    def __init__(self, container)
        self.value = container.GetData()
        ................................
        container.SetData(9)
        Two(container)

class Two:
    def __init__(self, container)
        self.value = container.GetData()
        ................................
        container.SetData(19)
        ................................

class Three:
    def __init__(self, container)
        self.container = Singleton()
        ................................
        container.GetData() # will return value 19 only
        container.SetData(19)
        ................................

if __name__ == "__main__":
    container = Singleton()
    container.SetData(9)
    One(container)

Please comment on my approach and modifications required if any. This code will be pushed in production box - so I want to double check my implementation.

I don't see why you need a singleton. You are already passing around container references as parameters. So, if you only create one container in __main__ and pass that to the class instances, they will all be accessing the same container.

Instead of calling Singleton() in Three.__init__ you would simply do the usual thing:

class Three:
    def __init__(self, container)
        self.container = container


Parse sentences and calculate mathematical results Content of task:

  Given a string of words and numbers. Extract the expression including:

  the operator: either addition or subtraction
  the two numbers that we are operating on

  Return the result of the calculation.

  Example:

  "Panda has 48 apples and loses 4" returns 44

  "Jerry has 34 apples and gains 6" returns 40

  "loses" and "gains" are the only two words describing operators.

  Should be a nice little kata for you :)

  Note: No fruit debts nor bitten apples = The numbers are integers and
  no negatives

  source: codewars.com

I want to optimise so ugly code:

def calculate(string)
  operator = '-' if string.split(' ').include? 'loses'
  operator = '+' if string.split(' ').include? 'gains'
  sum = 0
  count = 0
  n1 = 0
  n2 = 0
  string.split(' ').map do |s|
    if s.to_i != 0 && s.to_i.is_a?(Numeric)
      if count == 0
        n1 = s.to_i
        count += 1
      else
        n2 = s.to_i
      end
    end
  end
  n1.method(operator).(n2)
end

This code nice works(all tests was passed). But I don't want a large number of variables. Maybe have you some advice how to optimise this code? Thank you in advance.

The problem is that you are manipulating text without using Regular Expressions.

Regular expressions are a tool made specifically for text manipulation (search, delete, replace ... ) and are extremely good at their job.

The over-complication you experience comes from the use loops, conditionals and arithmetic to manipulate strings.

Here is a solution using regular expressions:

def text_calculation(text)
  a, b = text.split(/[^0-9]/).reject(&:empty?).map(&:to_i)
  text.include?("gains") ? a + b : a - b
end

First line explanation

text.split(/[^0-9]/)

This splits the text at every character that is not a digit (^ negates and [0-9] means from 0 to 9.

The output is this list:

["", "", "", "", "", "", "", "", "", "", "48", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "", "4"]

Almost what we want, we just need to remove (reject) the empty strings (empty?):

.reject(&:empty?)

So now we have ["48", "4"] that we must convert to integers before we can do arithmetic on, map applies a function to each item of a list and to_i converts to integer so .map(&:to_i)

Second line explanation

We add if we find "gains" else subtract (condition ? if_true : if_false is the Ternary operator)

.scan

We need to find the parts of the string that match a given regex, given that the regex /\d+\ matches all the subsequent strings of digits.

We must "find all regex matches in ruby", searching Google for such a task lands us on StackOverflow reveals that .scan is exactly what we can use. (The second Stack Overflow example is even about searching digits in a string!)

def text_calculation(text)
  a, b = text.scan(/\d+/).map(&:to_i)
  text.include?("gains") ? a + b : a - b
end

This code is more direct than splitting and filtering.


Better way to write list of places program I wrote some code using meteor, which is working, but I don't know if I did it the best way.  I'm asking for opinions on other ways to perform this task.

It's just a list of places.  You can fill the textfield with a place and it'll appear on the list.  You have an option to remove the place by clicking on the "x".

Very simple, but I'm new to meteor.

place_list.js

   Places = new Meteor.Collection('placesNew');
   if(Meteor.isClient){
     Template.places_list.places = function(){
       return Places.find({},{sort:{name: 1}});
      console.log(Template.place_list.place);
     }

   Template.places_list.events({
     'blur .place': function (obj) {
       Places.insert({name: obj.target.value});
     },
     'click .plc_remove': function(obj){
       Places.remove( obj.toElement.attributes['data-ref'].value);
     }
   });
}

place_list.html

  <title>Place List</title>

Place List :

  {{> places_list}}

<template name="places_list">
  <input type="text" placeholder="Insert a place here" class="place" />

  {{#each places}}

 {{ name}} | x |

  {{/each}}

</template>

Great start! You're very close. I changed things around a little bit, and added an extra template and a separate event map for that template. This should make it easier to understand, and it means you don't have to store the place's id in a data attribute. Check it out:

Places = new Meteor.Collection('places');

if(Meteor.isClient) {
    Template.places_list.places = function () {
        return Places.find({}, { sort: { name: 1 } });
    }

    Template.places_list.events({
        'keypress #place': function (evt) {
            if (evt.which === 13) {
                // The user pressed enter
                Places.insert({ name: evt.currentTarget.value });
                place.currentTarget.value = ''; // Reset the text input
            }
        }
    });

    Template.place_item.events({
        'click .plc_remove': function () {
            Places.remove(this._id); // Access the document fields with this.foo
        }
    });
}

And the HTML:

    <title>Place List</title>

Place List :

    {{> places_list}}

<template name="places_list">
    <input type="text" placeholder="Insert a place here" id="place" />

        {{#each places}}
            {{> place_item}}
        {{/each}}

</template>

<template name="place_item">

 {{ name}} | x |

</template>


Create edges from cells I have a (large) number of nodes and (triangular) cells, e.g.,

cells_nodes = [
    [0, 3, 1],
    [0, 2, 3]
    ]

This example represents the small mesh

(Please call me Picasso from here on out.)

Now, I would like to create the edges for these meshes (in red above), specifically edge–node and edge–cell relations. For the above example, the result would be

[[0], [1], [1], [0, 1], [0]]  # edge -> cells
[[1, 3], [2, 3], [0, 2], [0, 3], [0, 1]]  # edge -> nodes
[[0, 3, 4], [1, 2, 3]]  # cell -> edges

The problem I wasn't able to solve to my satisfaction is to identify the individual edges. Note that [1, 2] is the same edge as [2, 1]. Using frozensets works, but is perhaps overkill. This code

import numpy

def create_edges(cells_nodes):
    from collections import defaultdict
    d = defaultdict(list)
    # Loop over lists all pairs of nodes, i.e., all possible edges.
    cells_edges = []
    for local_edges in [[0, 1], [1, 2], [2, 0]]:
        # Put those into a list of frozensets so [21, 54] == [54, 21].
        cells_edges.append(
                map(frozenset, cells_nodes[:, local_edges])
                )
        # Store the edges in a dictionary, store the corresponding cell too.
        for k, f in enumerate(cells_edges[-1]):
            d[f].append(k)

    edges_nodes = numpy.array(map(list, d))
    edges_cells = list(d.values())

    edge_indices = dict(zip(d.keys(), range(len(d))))
    cells_edges = numpy.vstack(
        [[edge_indices[edge] for edge in c] for c in cells_edges]
        ).T

    return edges_nodes, edges_cells, cells_edges

if __name__ == '__main__':
    import meshzoo
    vertices, cells = meshzoo.rectangle.create_mesh(
            0.0, 1.0,
            0.0, 1.0,
            2, 2
            )
    edges_nodes, edges_cells, cells_edges = create_edges(cells)

    print(vertices)
    print
    print(cells)
    print
    print(edges_nodes)
    print
    print(edges_cells)
    print
    print(cells_edges)

works, but it too slow. The applications of map, and likely the explicit loops are killing the performance.

$ python -m cProfile -o program.prog e.py
$ snakeviz program.prog

Any idea of how improve this?

Honestly? It looks pretty straightforward. Everything in your code screams you've done this before. Good!

You're adhering to the standards of Python (the PEPs). The imports are at the exactly right places. The code is readable, easy to understand and does things the easiest way possible. In other words: it's Pythonic.

You could consider wrapping your print statements in main into a template so you'll only need one print call. However, since you only print once, I'm not sure how useful this would be in the end.

While your code is easy to understand, you might want to make the naming even better. You got 5 variables with the term edges in it, not even counting main where you use some of the exact names again. You're also overwriting cells_edges after the for after using it as a temporary variable. That's probably not the most maintainable way.

Single-character variable names should be avoided, even if they appear to be obvious.

d[f].append(k)

Yea, not a good idea. c, d, f and k should be renamed.


Functional Programming: int to bignum conversions I'm doing an exercise where I need to convert int to bignum in OCaml, for example:

123456789 -> {neg:false; coeffs:[123;456;789]}

There were a lot of things going inside my head when writing the function, and it was a load of fun. However, I'm not really sure if this is the cleanest, and fastest way to do this.

type bignum = {neg: bool; coeffs: int list};;
let base = 1000;;

let fromInt (n: int) : bignum =
  let rec aux num:int list=
    if num==0 then
       []
    else
       abs (num mod base) :: aux (num/base)
  in
  {neg=n<0; coeffs=List.rev (aux n)}
;;

fromInt turns an int to a bignum. Things to note:

I decided to mod the number with the base, and then divide the number
by the base and use that as the input. This is so I can avoid having to do exponents on ints. Was actually pretty excited when I realized that.
Used abs on (num mod base) every recursive call. Do you think there is a cleaner way of doing this? The the reason I didn't abs it on the first call was because (-max_int) <> min_int
As you can see I'm using List.rev because I do it in reverse order. This is so I can avoid having snoc lists. I'm not really sure if it's the only way though so feel free to note me.

And here is toInt, which turns a bignum into an int.

let toInt (b: bignum) =
  let rec aux (b:int list) (p:int)=
     match b with
     |hd::[]-> Some (hd*p)
     |hd::tl-> (match aux tl (p*base) with
        |Some c-> let bint = (hd*p) in
                  if bint < hd then None
                  else Some (bint+c)
        |None -> None)
     |[]->Some 0
  in
  let {neg=n; coeffs=x} = b in
  let c = List.rev x in
     match aux c 1 with
     |Some c-> if n then Some (-c) else Some c
     |None -> None
;;

I'm particularly not fond of the nested match statements, and there is probably still room for cleanup. Is there?

num==0 is misleading, use num=0 (although it has the same meaning for integers). Also, fromInt is spread over too many lines for my taste, I'd prefer the following choice of line breaks:

let fromInt (n: int) : bignum =
  let rec aux num : int list =
    if num=0 then []
    else abs (num mod base) :: aux (num/base) in
  {neg=n<0; coeffs=List.rev (aux n)}

This is fine for short lists of coefficients. In general tail recursion is preferred. I drop type annotations below. I add type annotations during debugging if I start getting confusing type errors, other than that I put types of functions in the interface (.mli).

let fromInt n =
  let rec aux acc num =
    if num=0 then acc
    else aux (abs (num mod base) :: acc) (num/base) in
  {neg=n<0; coeffs=aux n}

General note. I use the following kinds of line breaks with if..then..else depending on what fits in 80 columns and looks nice.

if test then blah
else blah

if test
then blah
else blah

if test
then
  blah
else
  blah

Now on to toInt. First, the way I would format it:

let toInt b =
  let rec aux b p =
    match b with
    | [] -> Some 0
    | hd::tl ->
      match aux tl (p*base) with
      | Some c->
        let bint = hd*p in
        if bint < hd then None
        else Some (bint+c)
      | None -> None in
  let c = List.rev b.coeffs in
  match aux c 1 with
  | Some c -> if b.neg then Some (-c) else Some c
  | None -> None

Again, it is interesting to explore the tail-recursive variant. I use my preferred style.

let toInt b =
  let rec aux acc p = function
    | [] -> Some acc
    | hd::tl ->
        let bint = hd*p in
        if bint < hd then None
        else aux (bint + acc) (p*base) tl in
  let c = List.rev b.coeffs in
  match aux 0 1 c with
  | Some c -> if b.neg then Some (-c) else Some c
  | None -> None


consolidate table td and th query I have two statements that look like this:

$('table tr td:nth-child(' + columnNumber + ')').toggle();
$('table tr th:nth-child(' + columnNumber + ')').toggle();

Is there a way to combine these two statements?

A comma.

$('table tr td:nth-child(' + rowNumber + '), table tr th:nth-child(' + rowNumber + ')').toggle();

jQuery's selectors are the same as CSS (with some stuff added here and there), so all the usual CSS selector rules apply:

$("div, h1")  // select div and h1 elements
$("div h1")   // select h1 elements that are nested somewhere within a div
$("div > h1") // select h1 elements that are direct descendants of a div

Etc.

By the way, rowNumber doesn't make sense; the code will select a given column, not a row.


Aid in making a clock code more modular Can someone help in making this clock code more modular so I will be able to use it in other bigger projects for example: turn into a c function. Here is the code:

#include <stdio.h>
#include <time.h>
#include <windows.h>
COORD coord = {0, 0};
void gotoxy (int x, int y)
{
    coord.X = x; coord.Y = y; SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE), coord);
}
void delay(unsigned int mseconds)
{
    clock_t goal = mseconds + clock();
    while (goal > clock());
}
void showClock(int x_1, int x_2, int x_3,int y){
    long i = 0;
    clock_t now = 0;                 int interval = 1;
    int elapsed = 0;
    int min=0,MIN=0,hrs=0,sec=0;
    int d=0,f=0;
    now = clock();
    for(i = 0L ;  ; i++){
        elapsed = (clock()-now)/CLOCKS_PER_SEC;
        if(elapsed>=interval){
            interval += 1;
            if(elapsed%60==0){
                min=elapsed/60;
                d=60*min;
                if(min%60==0){
                    hrs=min/60;
                    f=60*hrs;
                }
            }
            sec=elapsed-d;
            MIN=min-f;
            if(hrs<10){
                gotoxy(x_1,y);printf("0%d",hrs);
            }else{
                gotoxy(x_1,y);printf(":%d",hrs);
            }
            if(min<10){
                gotoxy(x_2,y);printf(":0%d",MIN);
            }else{
                gotoxy(x_2,y);printf(":%2d",MIN);
            }
            if(sec<10){
                gotoxy(x_3,y);printf(":0%d",sec);
            }else{
                gotoxy(x_3,y);printf(":%2d",sec);
            }
        }
    }
}
int main()
{
    showClock(2,4,7,4);
    return 0;
}

General comments

You should read the manual pages on time(), which gives you the time in seconds since 1970, library functions such as localtime, strftime and ctime that can be used to present time values and the sleep() function, which sleeps the process for a specified
  number of seconds.

Also:

variables and structure fields are normally not all-uppercase (X,Y,MIN)
put a single definition or expression on each line
leave a blank line between functions.
printf has many formatting options.  Of relevance here is the "%02d"
format string which prints an integer in 2 places, zero-padded if necessary

So for 1, it prints '01' and for 10 it prints '10'.
I prefer to see a space after while and if etc. and spaces around
operators (!=, %, / etc)
make all functions static where possible (in this program, all except
main)

- your function delay is not a good way of delaying, as it ties up the processor.  Better use sleep

Function showClock

I presume you wanted to call delay, but you didn't.  This function loops
continually using 100% of CPU time.  Better would be to use sleep() which
lets other processes run. So sleep(1) would sleep for 1 second.
using time() would be better than clock(), as you want intervals in
seconds
the single printf format "%02d:%02d:%02d" does what you want without pasing
three x-coordinates to the function
most of the rest is unnecessary if you use sleep and time and the other library functions to present the time interval