Creating Docker network with REST API using Python Django This solution is to create a docker network using Docker supported APIs. For achieving this, I am using Python and Django framework for providing RESTful APIs.

cat network.py
from django.http import JsonResponse
import requests
import json

HEADERS = {"Content-Type" : "application/json", "Accept" : "application/json"}

def network_details():
    response = requests.get("http://127.0.0.1:6000/networks")
    #table = [[Name, Id, Driver, Subnet, Gateway]]
    table = []
    details = {}
    for network in response.json():
        details["Name"] = network["Name"].encode("utf-8")
        details["Id"] = network["Id"].encode("utf-8")[:12]
        details["Driver"] = network["Driver"].encode("utf-8")

        #to check if dict is Null
        if network["IPAM"]["Config"].__len__() == 0:
            details["Subnet"] = "none"
            details["Gateway"] = "none"
        else:
            ip_config = network["IPAM"]["Config"][0]

            details["Subnet"] = ip_config["Subnet"]
            details["Gateway"] = ip_config["Gateway"]

        table.append(details)
        #intializing to dict to None as scope is outside the for loop as well
        details = {}
    return table

def network_info(request):
    if request.method == "GET":
        return JsonResponse(network_details(), safe=False)

def create_nw(content):
    response = requests.post("http://127.0.0.1:6000/networks/create",
                            content, headers = HEADERS )
    return response.json()

def create_network(request):
    if request.method == "POST":
        try:
            response = create_nw(request.body)
            #explicitly looking for Warning or message to know the status
            if "Warning" in response:
                return JsonResponse("Success, Network created with ID: {}".
                                   format(response['Id'].encode('utf-8')[:12]),safe=False)
         elif "message" in response:
                return JsonResponse("Error : {}".
                                   format(response.values()[0]),safe=False)
        except ValueError:
        return JsonResponse("Error : Improper JSON format provided", safe=False)

URLs information:

cat urls.py
from django.conf.urls import url

from . import views, images, containers, network

urlpatterns = [
              url(r'images', images.images_info),
              url(r'containers', containers.active_containers),
              url(r'network/create', network.create_network),
              url(r'network', network.network_info),
              url(r'^$', views.index, name='index'),
              ]

RESTful API to the solution:

curl -X POST <****>/network/create -d @create_network.json --header "Content-Type: application/json" | jq

Can someone suggest if this the right way to build an application using Django framework?

I am not looking for the appropriate/optimized solution for the idea I want to work on. I am looking for your suggestions how can I improve in my coding skills. The reason for selecting Django is to get hands on the framework.

Just for this .. is this all necessary? You could use the flask application , or mapping the each route to a script..

Short answer : No , there are others ways to build what you want that django is not more appropriate.

Long answer

First point

In your methods you are handling the permited methods at hand , the django have the decorators that handle this:

Allowed http methods
https://docs.djangoproject.com/en/1.11/topics/http/decorators/#allowed-http-methods

Some methods would:

@require_http_methods(["GET"])
def list_containers(request):
    pass

@require_http_methods(["GET"])
def list_images(request):
    pass

@require_http_methods(["GET","POST"])
def networks(request):

    if request.method == "GET":
        # retrieve networks and return them

    if method.method == "POST":
        # create network

urls.py

url(r'container/s', network.list_containers),
url(r'images/', network.list_images),
url(r'networks/', network.networks)

P.s : The views isn't following your structure , but is easily changed.

Second point

The mainly problem with your code is the routes: network/ and network/create to REST definition , the correct would be the route /networks receive the GET and POST and perform different actions ( retrieve and create respectively )

You could use the Django Rest Framework[1] ( the main framework to build restful application in django )

The DRF handle all these: method , routes , views ( viewsets ).

But if django is the bigger to this , you could use the flask framework and use the your routes and the restful frameworks that solves your problem too.

[1] http://www.django-rest-framework.org/

[2] http://flask.pocoo.org/ ( Framework )

[3] https://flask-restful.readthedocs.io/en/0.3.5/ ( FlaskRestful )


Search menu with multiple options if(selectedGroup == 0 && model.selectedParameterItem != null)
    {
        foreach (var i in model.selectedParameterItem)
        {
            parameters.Add(db.ParametersMethods.Where(f => f.ID == i).FirstOrDefault());
        }
        methods.AddRange(methodlogicclass.GetMethodFromParameters(parameters));
        ViewBag.methods = convertMethod(method);

        return View(methodModel);
    }

    else if (selectedGroup !=0 && model.selectedParameterItem == null)
    {
        methods = db.Methods.Where(f => f.MethodGroupID == selectedGroup).ToList();
        ViewBag.methods = convertMethod(methods);

        return View(methodModel);
    }

    else if(selectedGroup != 0 && model.selectedParameterItem != null)
    {
        List<Methods> groupMethods = db.Methods.Where(f => f.MethodID == selectedGroup).ToList();
        foreach (var i in model.selectedParameterItem)
        {
            parameters.Add(db.ParametersMethods.Where(f => f.ID == i).FirstOrDefault());
        }
        List<Methods> parameterMethods = methodlogicclass.GetMethodsFromParameters(parameters);

        foreach(var methods1 in groupMethods)
        {
            foreach(var methods2 in parameterMethods)
            {
                if(methods1.ID == methods2.ID)
                {
                    methods.Add(method2);
                }
            }
        }

        ViewBag.methods = convertMethods(methods);

        return View(methodModel);
    }

So I'm trying to create a searchmenu, with multiple options that don't necessarily have to be selected. However, search results obviously have to meet every option selected.

What would be the best way to write code like this, with the least amount of duplication, while keeping into account that model.selectedParameterItem (for instance) can be null and thus can't be used. Right now the code isn't closed for modification at all, which is far from optimal.

WHat would be best practice in a case like this? Check for each element if it is null, then get the results for each of them seperatly, then merge all the results?

Code explained:

Search by group => search for methods by group
Search by parameter => search for methods by parameters
Etc, more will be added
When searching for both, only return the methods that meet both group and parameter requirement (and in the future, other requirements).
Group can be empty (0), list of parameters can be empty (null).
Future added requirements will be able to be empty (null)

Use English language for naming. .NET uses English for naming. You have mix of languages, and code looks like obfuscated for those who don't speak German.
Use descriptive names. I don't know German, but wvl, wek1, wek2 is definitely not very descriptive.
You have mix of presentation-related and data access code. You should split this code.

So, the first part is your controller code. It should look like technical specification when you read it. Let's try to do that

var methods = GetMethods(selectedGroupId, model.selectedParameterItem);
var model = ConvertMethodsToModel(methods);
return View(model);

Pretty clean. Now lets think about GetMethods method. It's purpose is returning some methods depending on what was found in methods group and parameter methods. Btw it can be method of another class (e.g. some service)

private IEnumerable<Method> GetMethods(int groupId, IEnumerable<int> parameterIds)
{
   var none = Enumerable.Empty<Method>();
   var groupMethods = (groupId == 0) ? none : GetMethodsByGroup(groupId);
   var parameterMethods = (parameterIds == null) ? none : GetParameterMethods(parametrIds);

   if (!parameterMethods.Any())
       return groupMethods;

   if (!groupMethods.Any())
       return parameterMethods;

   var parameterMethodsFromGroup =
        from pm in parameterMethods
        join gm in groupMethods on pm.Id equals gm.Id
        select pm;

   return parameterMethodsFromGroup;
}

And last part - those GetMethodsByGroup and GetParameterMethods (which could be moved to corresponding repositories):

private IEnumerable<Method> GetMethodsByGroup(int groupId)
{
    return db.Methods.Where(m => m.MethodGroupID == groupId).ToList();
}

Getting parameter methods:

private IEnumerable<Method> GetParameterMethods(IEnumerable<int> parameterIds)
{
    var parameterMethods = db.ParametersMethods.Where(pm => parameterIds.Contains(pm.ID));
    return methodlogicclass.GetMethodFromParameters(parameterMethodss).ToList();
}


Mapping/parsing database table data to output data I have currently implemented this simple example of a database to output for a REST API to show the way information is being parsed, mapped and serialized starting from database data converting it to JSON. This solution currently works fine, but the problem comes from the fact that it makes use of PHP tricks (like calling a function from a string) which I really do not approve of. Do you think there is a better way/pattern to implement/split this solution?

class DataMapper {
    /**
     * @var array
     */
    private $rawData;

    /**
     * @var array
     */
    private $parsedData;

    /**
     * @var array
     */
    private $mapping = [
        'dbField1' => 'outputField1',
        'dbField2' => ['outputField2' => ['double']],
        'dbField3' => ['outputField3' => ['double', 'dollars']]
    ];

    public function __construct(array $data) {
        $this->rawData = $data;
        $this->mapData();
    }

    /**
     * @param $value
     * @return mixed
     */
    private function double($value) {
        return $value * 2;
    }

    /**
     * @param $value
     * @return string
     */
    private function dollars($value) {
        return $value . "$";
    }

    // Yes, I know the methods shouldn't be here but it rather be decoupled in another class and
    // injected with a DI but I wrote it like this just as an example
    private function mapData() {
        foreach($this->mapping as $dbFieldName => $content) {

            if(isset($this->rawData[$dbFieldName])) {

                if(is_string($content)) {
                    $this->parsedData[$content] = $this->rawData[$dbFieldName];
                } else
                if (is_array($content)) {
                    $frontFieldName = key($content);
                    $functionsArray = reset($content);
                    $value = $this->rawData[$dbFieldName];

                    foreach($functionsArray as $functionName) {
                        $value = $this->{$functionName}($value);
                    }
                    $this->parsedData[$frontFieldName] = $value;
                }
            }
        }
    }

    /**
     * Simple getter
     * @return array
     */
    public function getParsedData() {
        return $this->parsedData;
    }
}

// Usually the data comes from the DB so it would be in some kind of Row Set structure
$data = ['dbField1' => 1, 'dbField2' => 1, 'dbField3' => 1];
// The data is handed to the Holder which parses it
$dm = new DataMapper($data);
// The data is parsed so it can be
echo json_encode($dm->getParsedData());

You can experiment with it here.

I would highly appreciate it if you could give me pointers to how this could be made better programming to interfaces instead of simple strings; using composition, dependency injections, and maybe separation of the mapper functions in some abstract factories.

The only thing I would change is the mapping definition, e.g. separate the configuration from the actual logic so you don't have to write many mappers, but can use one with multiple configurations.

Quoting http://martinfowler.com/eaaCatalog/metadataMapping.html

  Much of the code that deals with object-relational mapping describes how fields in the database correspond to fields in in-memory objects. The resulting code tends to be tedious and repetitive to write. A Metadata Mapping allows developers to define the mappings in a simple tabular form, which can then be processed by generic code to carry out the details of reading, inserting, and updating the data.

The dollars method probably shouldn't be in your mapper though, because it is formatting the data. Move this to the presentation layer. If you want to map to money, introduce a Money object.

You can spice it up by also introducing a TypeConverter thing which then knows how to handle certain types from the ColumnDefinition, e.g. something like

class FooMapDefinition implements IteratorAggregate
{
    private $map = [
        'someProperty' => [
            'mapTo': 'propertyName',
            'mapAs': Money::class
        ],
        // … more definitions
    ];

    public function getIterator()
    {
        return this new ArrayIterator($this->map);
    }

}

Then inject a TypeConverterFactory into the mapper. When you map the properties, you let the factory create a converter from the class name in mapAs, push the value from the source data through it and then map it to your entity.


Bash “Rotating” MySQL backup I'm quite a novice in shell scripting so far, but i'm learning and getting quite comfortable in writing shell scripts, but i have a lot to learn.

The snippet at the bottom of the page creates backups of a certain users databases.

The script checks first if there is more than 7 compressed .gz files in a folder.

Fetches oldest gz archive and removes it.

Checks for latest .sql backup and compresses it with gzip

Creates a new .sql dump

The script is run once a day

How would you write this differently?

#!/bin/bash

BACKUPPATH=/var/mysql_backup/
mkdir -p $BACKUPPATH
if [ $(ls -l $BACKUPPATH*.gz 2>/dev/null | wc -l) -gt 7 ]; then
        OLD=$(find $BACKUPPATH -type f -name '*.gz' -printf '%T+ %p\n' | sort | head -n 1 | awk '{print $2}')
        rm $OLD # Removing oldest compressed archive
fi

if [ $(ls -l $BACKUPPATH*.sql 2>/dev/null | wc -l) -eq 1 ]; then
        LATESTLOG=$(find $BACKUPPATH -type f -name '*.sql' -printf '%p\n' | head -n 1)
        gzip $LATESTLOG
fi

/usr/bin/mysqldump -u db_user -p'password' --single-transaction  --all-databases > $BACKUPPATH$(date "+%Y.%m.%d-%H.%M.%S")_dump.sql

I'd do this:

#!/bin/bash

backuppath=/var/mysql_backup
mkdir -p "$backuppath"

shopt -s nullglob

# Remove old compressed archive
backups=( "$backuppath"/*.gz )
if [[ ${#backups[@]} -gt 0 ]]; then
    stat -c "%Y %n" "${backups[@]}" |
      sort -rn |
      sed -e '1,7d' -e 's/^[0-9]\+ //' |
      xargs -r rm
fi

# Compress all the logs
logs=( "$backuppath"/*.sql )
if [[ ${#logs[@]} -gt 0 ]]; then
    gzip "${logs[@]}"
fi

/usr/bin/mysqldump -u db_user -p'password' --single-transaction --all-databases \
    > "$backuppath/"$(date "+%Y.%m.%d-%H.%M.%S")_dump.sql

Notes:

don't use ALLCAPSVARS -- leave those for the shell. I've seen people accidentally do PATH=... and then wonder why their script is broken.
don't parse ls -- I'm using arrays to hold the results of the glob expansion, and checking how many elements are in the array.
always quote variables, except when you know exactly why not. See https://unix.stackexchange.com/q/171346/4667
I'm assuming you want to compress any log.sql file, not just the first found
I'm assuming you want to keep 7 compressed logs and remove any older ones.
I'm assuming you have the GNU coreutils (xargs -r, those stat options)


Java Stream wants to return only failure list I have an object delivery template, which I want to delete based on a name. String array which contains delivery template name is taken as an input parameter. I use Java Stream to iterate the list. Please find the code below.

    public static List<String> deleteDeliveryTemplates(String... deliveryTemplateNames) {
    List<String> failureList = new ArrayList<String>();
    Stream<String>deliveryTemplateNamesStream= Stream.of(deliveryTemplateNames);
     deliveryTemplateNamesStream.forEach(deliveryTemplateName ->{
        try
        {
            deleteDeliveryTemplate(deliveryTemplateName);
        } catch (Exception e)
        {
            failureList.add(deliveryTemplateName);
        }
    });
    return failureList;
}

Is there a better way in JAVA 8 to return the failure list using stream. For example: using map function of Stream

Just because you can do it in streams, that doesn't mean you should. What you really want is the following:

public static List<String> deleteDeliveryTemplates(String... deliveryTemplateNames) {
    List<String> failures = new ArrayList<>();
    for (String templateName : deliveryTemplateNames) {
        try {
            deleteDeliveryTemplate(templateName);
        } catch (Exception e) {
            // LOG THIS EXCEPTION!
            failures.add(templateName);
        }
    }
    return failures;
}

This code is clean, simple, does one thing. Note that I removed the overspecification of generics on the right-hand-side of the failureList declaration and changed the names of your local variables a bit to make use of the surrounding context.

I also slightly reformatted the brace positions in try-blocks to be more in line with the usual Java conventions. As long as you're consistent, there's nothing wrong with your style though.


Is it bad when my code is throwing exceptions, but doesn't crash during runtime? Here's part of code in Java for battleships game I'm working on:

    lblGrid2 = new JLabel(new ImageIcon(myPicture));
    lblGrid2.addMouseListener(new MouseAdapter() {
        @Override
        public void mouseClicked(MouseEvent e) {
            for (int i = 0; i < shipList.size(); i++)
                if (shipList.get(i).isActive())
                    shipList.get(i).place();
        }
    });
    lblGrid2.addMouseMotionListener(new MouseMotionAdapter() {

        @Override
        public void mouseMoved(MouseEvent m) {

            for (int i = 0; i < shipList.size(); i++) {
                if (shipList.get(i).isActive()) {
                    shipList.get(i).moveShip(m.getX(), m.getY());
                }
            }
        }
    });
    lblGrid2.setText("");
    lblGrid2.setBounds(40, 40, 401, 401);
    panel.add(lblGrid2);

When placing ships on board. this listens for mouse movement and should move ship image accordingly, but that happens only after the client and server are connected and there is an active ship that can be placed. If that's not the case, for example I haven't connected to the server yet, every mouse move throws an exception.

Is this something I should be concerned about?

Yes, absolutely, you should be concerned.

Exceptions should be exactly that, exceptional. and should never be part of the regular code path. If there are circumstances that you know would be normal (like not yet being connected to a server) then you should code in a  condition that represents that situation, and make your code a conditional-execution path. For example, in your code, I would have an AtomicBoolean that represents the 'connected' state, and then only process mouse-events when connected.

As a side-note, Exceptions in Java are very 'expensive'. Almost all Java VM's (OpenJDK, Oracle JDK, IBM JDK, Android JDK, etc.) need to inspect the entire call-graph of the code when an exception happens, so that it can build up the stack trace for the exception. In all the VM's I know, that requires that:

The entire VM thread is 'suspended' so that the call-graph does not change
all threads are 'paused' <- looking for evidence of this still
the call stack is inspected and walked up.
all references on the call-stack are chased back in to the source (get line numbers, etc.)

This is very expensive to process. Using exceptions as part of the normal program flow is a bug, and preventing exceptions with conditional statements is always a good idea. For example, the following code example is terrible:

int val = 0;
boolean isNumber = false;
try {
   val = Integer.parseInt(somestring);
   isNumber = true;
} catch (NumberFormatException nfe) {
   // ignore not-numbers, and leave isNumber to be false
}

I see code like the above all over the place.... and it is broken. In one example, I have seen a 50% performance improvement on a CSV reading program by changing code like that to be:

int val = 0;
boolean isNumber = false;
if (someString.matches("\\d+")) {
    try {
       val = Integer.parseInt(somestring);
       isNumber = true;
    } catch (NumberFormatException nfe) {
       // ignore not-numbers, and leave isNumber to be false
    }
}

Even though the code does extra work to pre-validate that the string contains digits, it is faster! (and is even faster if you precompile the Match pattern...!)

So, in your code, I would have:

private final AtomicBoolean isConnected = new AtomicBoolean(false);

and then, in your code:

lblGrid2.addMouseMotionListener(new MouseMotionAdapter() {

    @Override
    public void mouseMoved(MouseEvent m) {

        if (!isConnected.get()) {
            return;
        }

        for (int i = 0; i < shipList.size(); i++) {
            if (shipList.get(i).isActive()) {
                shipList.get(i).moveShip(m.getX(), m.getY());
            }
        }
    }
});

Some discussions about Java Stack traces and exceptions:

Longjumps Considered Inexpensive
How slow are Java exceptions?


Parsing a u8 separated vector of FromStr from a BufRead I need to read a vector of e.g. integers from a stream, usually from stdin but sometimes also from a file. The input is always less than a megabyte, and is sometimes separated by commas and sometimes by newlines.
I'm calling the code directly from my main function, so I'm more concerned with returning a human readable error rather than something that can be caught and recovered from programmatically, hence I'm using String as the error type.
pub fn parse_vector<T: BufRead, S: FromStr>(buf: T, sep: u8) -> Result<Vec<S>, String> {
    buf.split(sep)
        .enumerate()
        .filter_map(|(i, entry)| {
            let entry_nr = i + 1;

            let entry = match entry.map(String::from_utf8) {
                Err(e) => return Err(format!("Cannot read entry {}, {}.", entry_nr, e)).into(),
                Ok(Err(e)) => return Err(format!("Cannot read entry {}, {}.", entry_nr, e)).into(),
                Ok(Ok(v)) => v,
            };

            let trimmed = entry.trim();
            if trimmed.is_empty() {
                None
            } else {
                Some(
                    trimmed
                        .parse::<S>()
                        .map_err(|_| format!("Cannot parse entry {}: '{}'", entry_nr, entry)),
                )
            }
        })
        .collect()
}

The above code works as far as I can tell, but I'm wondering if there's a nicer way to deal with the mess in the match expression. The functions from_utf8 and split return different error types, but both implement Display so maybe there's some elegant way to eliminate some code duplication there that I don't know about (I'm new to rust, coming from c++).

What are my options to make this code more readable?
Is it considered bad practice to return String as an error type?

I'm calling the code directly from my main function, so I'm more
concerned with returning a human readable error rather than something
that can be caught and recovered from programmatically, hence I'm
using String as the error type.
[...]

Is it considered bad practice to return String as an error type?

Yes, it is generally considered bad practice to return Result<_, String> when the error returned is formatted from other errors.  The recommended approach is to define an enum and implement From, Display, and Error: (using the thiserror crate for simplicity)
use thiserror::Error;

#[derive(Debug, Error)]
pub enum Error {
    #[error("cannot read entry {entry_no}")]
    Io { entry_no: usize, source: std::io::Error },
    #[error("cannot read entry {entry_no}")]
    Encoding { entry_no: usize, source: std::string::FromUtf8Error },
    // ...
}

The above code works as far as I can tell, but I'm wondering if
there's a nicer way to deal with the mess in the match expression. The
functions from_utf8 and split return different error types, but
both implement Display so maybe there's some elegant way to
eliminate some code duplication there that I don't know about (I'm new
to rust, coming from C++).

Your options include:

methods on Option and Result, like Result::and_then and Option::ok_or_else;

the ? operator; and

using .zip(1..) instead of .enumerate() to get rid of that + 1,

plus potentially other things that didn't immediately come to mind.


Check if Binary Search Tree I wrote this entirely from scratch without any Googling/reading as part of an ongoing effort to improve my problem-solving skills.

Also, please note that the focus for my project was the problem solving steps rather than the code itself, which is why I have the comments. It is also why I manually set up the BST (this helped me visualize the problem in order to solve it). However, I am indeed asking for a code review.

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>
typedef struct Node {
      int data;
      struct Node* left;
      struct Node* right;
      struct Node* parent;
   } node;
bool checkBST(node *root){

    //Step 1, check if the node is not null *******
    //Step 2, check if left child exists **********
    //Step 3, if left child exists, compare its data to the root/current node's data and make sure that it is SMALLER **********
    //step 4, if the data is smaller, proceed, else return false ***********
    //step 5, if the root's right child exists, compare its data to the root/current node's data and make sure it is BIGGER *********
    //step 6, if the data is bigger, proceed, else return false *******

    //step 7, recurse *****
    //step 8, return true *****

    if(root == NULL){
        printf("root is null!\n");
        return false;
    }

    if(root->left != NULL){
        printf("root data: %d\nleft data: %d\n",root->data,root->left->data);
        if(root->data < root->left->data){
            printf("Not a BST: left is bigger\n");
            return false;
        }
        checkBST(root->left);
    }

    if(root->right != NULL){
        printf("root data: %d\nright data: %d\n",root->data,root->right->data);
        if(root->data > root->right->data){
            printf("Not a BST: right is smaller\n");
            return false;
        }
        checkBST(root->right);
    }

    return true;
}

int main(){
    node *n1, *n2, *root, *n4, *n5, *n6;

    n1 = malloc(sizeof(node));
    n2 = malloc(sizeof(node));
    root = malloc(sizeof(node));
    n4 = malloc(sizeof(node));
    n5 = malloc(sizeof(node));
    n6 = malloc(sizeof(node));

    //Set up data values
    n1->data = 10;
    n2->data = 20;
    root->data = 30;
    n4->data = 15; //modified this node to test for negative outcome
    n5->data = 50;
    n6->data = 60;

    //Set up children/parents
    root->parent = NULL;
    root->left = n2;
    root->right = n4;
    n1->left = NULL;
    n1->right = NULL;
    n2->parent = root;
    n2->left = n1;
    n2->right = NULL;
    n4->parent = root;
    n4->left = NULL;
    n4->right = n5;
    n5->parent = n4;
    n5->left = NULL;
    n5->right = n6;
    n6->parent = n5;
    n6->left = NULL;
    n6->right = NULL;

if(checkBST(root) == true){
    printf("This is a BST!");
}

    return(EXIT_SUCCESS);
}

You forgot to check the result of your recursing calls. Your steps are fine, if you meant the following in step 7

  Step 7: recurse and check result

However, even then, it's not the correct property of a binary search tree. Have a look at the following tree:

    5
   / \
  3   7
 / \
1   6

Does this hold for your steps? Yes. Does this hold for your algorithm, if you check the result of checkBST?

bool checkBST(node *root){
    if(root == NULL){
        return false;
    }

    if(root->left != NULL){
        if(root->data < root->left->data || !checkBST(root->left)){
            return false;
        }
    }

    if(root->right != NULL){
        if(root->data > root->right->data || !checkBST(root->right)){
            return false;
        }
    }

    return true;
}

Yes. But is it a binary search tree? No. You would never find 6. All nodes left of the root must be smaller, and all nodes right of the root must be greater.

Remember, a binary tree has the following property: if you want to find a value, you look at the current value. If the value you're looking for is smaller, you continue in the left tree. If the value is greater, you continue in the right tree. But you always look at one of those subtrees. Never both. In a balanced binary tree, you would only have to look at \$\log_2(N)\$ nodes to find your value.

I would link you to the Wikipedia section on Verification, but it already holds the solution. But here is a hint: carry along what numbers are allowed in your subtree.

So, to go back to your steps, you should do something like this:

check if the node is not null
check if left child exists

check that all elements in the left subtree are smaller than our current value

check if right child exists

check that all elements in the right subtree are greater than our current value

The steps 0 and 1.5 are missing. They handle the current value and the range of valid values. Coming up with them will be the crucial part.

By the way, you usually don't use parent in a binary tree. But that depends on your use-case. Also, you have memory leaks. For small examples, there is no harm not using malloc:

struct node n10, n20, n30, n15, n50, n60;

n10.data = 10;
n10.left = NULL
n10.right = NULL
n10.parent = &n20;

n20.data   = 20;
n20.parent = &root;
n20.left   = &n10;
n20.right  = NULL;

// ....


NodeJS Module Exports: Export multiple functions vs Exporting a single function To update parameters in our main.js file, which has a lot of configurable variables, we can do the following

A .

 module.exports.setVarA = function(val){
    VarA = val;
}
module.exports.setVarB = function(val){
    VarB = val;
}
...so on for VarC up to VarZ

B.

module.exports.setAllVariables = function(valA, valB.... valZ){
    VarA = valA;
    VarB = valB;

    ...
    VarZ = valZ;
}

Which one is a better approach to setting up variables in NodeJS? (faster, easier to maintain, not an antipattern)

Which one is a better approach to setting up variables in NodeJS? (faster, easier to maintain, not an antipattern)

Evaluating the merit of different options depends upon these factors:

How many total options are there now?
How likely are you to add more options as time goes?
Does the caller always set all the options at once? Or is there value to being able to set just some options
Does the caller usually just set one option?

Evaluating each of these:

Total number of options

First off, if you have lots of variables, then neither A or B are particularly practical.  A is not very practical because you have to make a separate API for every variable when they're all really just doing the same thing, but for a slightly different property.  That's generally not a good thing in an API design.

For lots of variables, option B ends up having to list out a lot of different values all in some precise order and that's not easy to use without having the doc page open every time you want to use it and it's not all that easy to read code that's written to use it either because the reader won't remember which position every single parameter is.

Extending to add more options in the future

Both A and B require modifying the API to extend and add more options.  Option A requires adding and documenting a new API entry point.  Option B requires adding and documenting a parameter and then making sure that prior code that doesn't include the new parameter isn't broken (by detecting the missing parameter on the receiving end of things).  Neither is ideal.

Does the caller always set all options at once

If so, option A is a pain.

Does the caller usually set just one option or a small number of options

If so, option B is a pain.

Do, what you find is that neither option A or option B is entirely flexible for different types of calling usage or extensibility or use with lots of options.

Option C

So, I'll propose using a single options object to be a lot more flexible for different types of calling usage and infinite extensibility without changing the API surface itself.  This is also trivial to maintain.  Let's call this option C.

// default values for options
let defaultOptions = {
    optionA: defaultValA,
    optionB: defaultValB,
    optionC: defaultValC,
    optionD: defaultValD,
    optionE: defaultValE
};

module.exports.setOptions = function(options) {
    // copy any properties from the options object to the default options
    //   overwriting default values with new values
    Object.assign(defaultOptions, options)
}

Or, in some cases, you want to prevent adding anything to defaultOptions that isn't already there (no additional properties allowed):

module.exports.setOptions = function(options) {
    // Copy any properties from the options object to the default options
    //   overwriting default values with new values
    // Only allow copying of properties that are already in defaultOptions
    for (let prop of Object.keys(options)) {
       if (Object.hasOwnProperty(defaultOptions, prop)) {
           defaultOptions[prop] = options[prop];
       }
    }
}

And, the caller would do something like this:

m.setOptions({
    optionC: "someNewValue",
    optionE: "anotherNewValue"
});

Where they can include only whatever options they want to set and they are named options so even if there were 1000 options and you just wanted to set 2 of them, it works quite cleanly.

For extensibility, option C wins every time because it can be extended by simply documenting that you now accept another property name without changing the surface of the API at all.

Option D

A fourth option which I will call option D, is to just have one "setter" function that takes a property name and a value.

// default values for options
let defaultOptions = {
    optionA: defaultValA,
    optionB: defaultValB,
    optionC: defaultValC,
    optionD: defaultValD,
    optionE: defaultValE
};

module.exports.setProp = function(propName, value) {
    defaultOptions[propName] = value;
    return this;
}

This allows you to have one single function that could even be chainable that can set any property.

m.setProp("optionC", "someNewValue").setProp("optionE", "anotherNewValue");

This is probably better than option A if everything is just property name and value because it's more extensible without modifying the API, but it's a pain to use if you're trying to set a lot of properties (where option C excels).

Review

For option A, it's really only practical or best if you have a small number of options (say less than 5) and the caller is typically not setting multiple options (typically only setting one option).  Even then, I probably still prefer option C as its infinitely extensible without adding more entry points to the API.

For option B, it's really only practical or best if you have a small number of options (say less than 5) and the caller is always setting all the options (or at most leaving off the last one or two).  Option B is complicated to use and read if there are lots of options (because you have to remember the right position in the parameter list for each option) and it's complicated to overload to only pass some options.  The best part of option B is that with a small number of options, it's probably the least typing for the caller, but that lesser typing means it's less self documenting than the object with named properties in option C.

For option C, it's infinitely extensible and flexible for any type of caller use from setting one property to setting hundreds of properties.  And, it's trivial to maintain, even as you add more options.  This is generally my preferred choice anytime I have 3 or more options or value extensibility or flexibility in how it's used by the caller.

For option D, I don't personally find it cleaner than option C, but it is flexible and extensible, though inefficient for setting a bunch of options.


Route factory which creates instances based on database context and model I am trying to bridge the gap between the database context and routes that are stored in the database.

The factory is designed so that when used in other assemblies, you are forced to use the database context which is known to contain the DbSet which can provide the required data to populate the route.

In the assembly in which the factory resides, you can specify the database context to use so that if more route types are created the route factory can provide different types of route to other classes within the assembly.

However, the idea initially behind the factory was that the assemblies which reference the assembly with the factory could also specify a database context or model as long as it inherits the base database context or model. I'm aware that making the methods internal prevents this, but the methods have been made internal as they may be removed if I refactor the way everything works.

The following is the class:

namespace Sapphire.Cms.Web.Routing
{
    using Sapphire.Cms.Data.Entity;
    using Sapphire.Cms.Models;
    using System;
    using System.Web.Routing;

    public class RouteFactory : IRouteProvider
    {
        public virtual RouteBase GetRoute(IRouteHandler routeHandler)
        {
            DbContextTypeWrapper<SapphireDbContext<SiteTree>, SiteTree> dbContextTypeWrapper = new DbContextTypeWrapper<SapphireDbContext<SiteTree>, SiteTree>();
            return GetRoute(dbContextTypeWrapper, routeHandler);
        }

        internal virtual RouteBase GetRoute(DbContextTypeProvider databaseContextTypeProvider, IRouteHandler routeHandler)
        {
            return GetRoute(databaseContextTypeProvider, typeof(SiteTree), routeHandler);
        }

        internal virtual RouteBase GetRoute(DbContextTypeProvider databaseContextTypeProvider, Type model, IRouteHandler routeHandler)
        {
            Type catchallRoute = typeof(CatchallRoute<,>).MakeGenericType(new Type[] {
                databaseContextTypeProvider.DbContextType,
                model
            });

            Type routeTypeWrapper = typeof(RouteTypeWrapper<>).MakeGenericType(new Type[] {
                catchallRoute
            });

            RouteTypeProvider routeTypeProvider = (RouteTypeProvider)Activator.CreateInstance(routeTypeWrapper);

            return GetRoute(routeTypeProvider, routeHandler);
        }

        private static RouteBase GetRoute(RouteTypeProvider routeTypeProvider, IRouteHandler routeHandler)
        {
            Object[] constructorParameters = new Object[]
            {
                routeHandler
            };

            return (RouteBase)Activator.CreateInstance(routeTypeProvider.RouteType, constructorParameters);
        }

        #region IRouteProvider Members
        RouteBase IRouteProvider.GetRoute(IRouteHandler routeHandler)
        {
            return GetRoute(routeHandler);
        }
        #endregion
    }
}

As you can see the class makes heavy use of reflection, which is bad for performance, however the intended use of this factory is that it should only ever have to get one route, a catchall route.

My questions are:

Based on what was said, should I keep the internal methods internal or make them public?
Should the factory be refactored to improve performance despite the intention being to only ever use it once per request?

Any particular reason you have all those using statements in the namespace itself?

This line:

DbContextTypeWrapper<SapphireDbContext<SiteTree>, SiteTree> dbContextTypeWrapper = new DbContextTypeWrapper<SapphireDbContext<SiteTree>, SiteTree>();

throws a gray-flag for me. Perhaps you should consider injecting the DbContextTypeWrapper into the RouteFactory? It seems to have an awful dependence on certain specifics, that can be abstracted away.

Another suggestion, would be to consider generics for it, considering you also depend on SiteTree as well.

The ideal solution would be an interface ISiteTree that you depend on, regardless of the SiteTree implementation. You could provide a default SiteTree implementation, or allow the additional assemblies to provide their own.

Likewise, the use of internal tends to lend way to some particular properties, for example, the behavior between assemblies can be different. This is particularly foul. You should consider either marking the internal methods public, or marking them private.

Depending on what you have access to, you should consider interfaces/generics again to allow for the DbContextTypeWrapper to vary a little between assemblies. By restricting the public implementation to SapphireDbContext, you also restrict it to only those DB types that SapphireDbContext supports. (If it only supports MSSQL, then you'll need an MSSQL server for each-and-every other project you use this in.)

Though I do not generally advocate the use of var, you could shorten some of your lines with it. (Especially the obscenely long ones.)


Remove duplicates in array This function sorts an array and returns the unique items. I'm trying to get it down to a readable 5 lines of code. So far I've renamed the variable cursor variable and inlined the assignments.
I'll appreciate fresh eyes.

Before

function uniques(array) {
    var result = [], val, ridx;
    outer:
    for (var i = 0, length = array.length; i < length; i++) {
        val = array[i];
        ridx = result.length;
        while (ridx--) {
          if (val === result[ridx]) continue outer;
        }
        result.push(val);
    }
    return result;
}

After

function uniques(array) {
    var result = [], val, cursor;
    outer:
        for (var i = 0, length = array.length; i < length; i++) {
        val = array[i], cursor = result.length;
        while (cursor--) {
          if (val === result[cursor]) continue outer;
        }
        result.push(val);
    }
    return result;
}

This function sorts an array and returns the unique items

Remove duplicates from the array using Array#filter
Sort the array using Array#sort

Code:

function uniques(arr) {
    // Remove the duplicate elements from the array
    return arr.filter(function (element, index, arr) {
        return arr.indexOf(element) === index;
    }).sort(function (a, b) {
        return a - b;
    });
}


3-Digit Narcissistic Numbers Program - Python Just wanted to share my Narcissistic Numbers Program with y'all (not sure if it's good, but I gave it my best). Hope you like it!
This program prints out all the narcissistic numbers from the smallest number and biggest number's range. Ex. in the 1-100 range, only the narcissistic numbers will show on the console.
Some of you may be wondering, "What the hell are narcissistic numbers??" Once you go through a number of examples, I'm sure they won't be as bad as they sound (no pun intended). Narcissistic numbers are also known as Armstrong numbers, though I'm going to use the term 'narcissistic' rather than 'Armstrong' to avoid being inconsistent and flipping back and forth.
Overview: Narcissistic numbers are numbers that are equivalent to the sum of the cubes of their digits (3-digit numbers). This definition's verbose structure confused me, but after looking through a couple of examples, it made much more sense.

Procedure: (this is only applicable to 3-Digit Numbers)

Pick a number: 153

FYI: I broke the definition down into my own steps.

I split the number up into its individual digits; 1, 5 and 3
or you can just identify which digits the number (153 in this case) is composed of; 1, 5 and 3

Both ways bear the same result, but I tend to over-complicate things, so you can choose which way to use.

Now that all the digits are split up, we need to cube them all (separately) like this:
[1⋅1⋅1] + [5⋅5⋅5] + [3⋅3⋅3]; I included the square brackets for clarity's sake.

If you're not using brackets, then make sure you're following the BEDMAS rules.
If you simply calculate the following in a calculator: 1⋅1⋅1 + 5⋅5⋅5 + 3⋅3⋅3, then the result you will get is: 1025.

I believe I have provided a thorough enough over-view as to what narcissistic numbers are. Now, I hope the code will be easier for many people to understand.
The code is as follows:
import time
initial_time = time.time()

# initializing 's_number' (the smallest number) and 'l_num' (the largest number)
s_number = 100
l_number = 500

for number in range(s_number, l_number + 1):

    # order of the numbers
    order = len(str(number))

    # initializing sum
    sum = 0

    var = number
    while var > 0:
        digit = var % 10
        sum += digit ** order
        var //= 10

    # setting restrictions for program
    if number == sum:
        print(number)

total_time = time.time() - initial_time
print('Time to complete task:',total_time)

If there's anything to change/add to make the program more efficient/better please don't hesitate to let me know. I would appreciate your insight on this project.

Nice solution, it's already efficient and easy to understand. Few general suggestions:

Avoid calling a variable sum: sum is a built-in function in Python, calling a variable sum prevents you from using such function.
time.perf_counter vs time.time: running your code I get 0.0 running time on Windows, which doesn't provide a useful information. Use time.perf_counter to get a better precision.
Make a function and return a result: if you wrap the code in a function it is easier to reuse and test. Additionally, instead of printing the numbers, add them to a list and return the final result. Printing only the result is typically faster than printing the numbers one by one.
Armstrong numbers: a narcissistic number is also called Armstrong number and here there is an efficient approach that I slightly modified to fit this question:
from itertools import combinations_with_replacement

def armstrong(n):
  numbers = []
  order = len(str(n))
  for k in range(1, order):
      a = [i ** k for i in range(10)]
      for b in combinations_with_replacement(range(10), k):
          x = sum(map(lambda y: a[y], b))
          if x > 0 and tuple(int(d) for d in sorted(str(x))) == b:
              numbers.append(x)
  return sorted(numbers)

Extending the definition of Narcissistic number to:

An n-digit number that is the sum of the nth powers of its digits

The Narcissistic numbers till 1000000 are:
1, 2, 3, 4, 5, 6, 7, 8, 9, 153, 370, 371, 407, 1634, 8208, 9474, 54748, 92727, 93084, 548834

Runtime:
             n=1_000_000      n=10_000_000
Original:     3.374 s            40.26 s
Armstrong:    0.036 s            0.098 s

Full code:
import time
from itertools import combinations_with_replacement

def original(n):
    numbers = []
    for number in range(1, n):
        order = len(str(number))
        total_sum = 0
        var = number
        while var > 0:
            digit = var % 10
            total_sum += digit ** order
            var //= 10
        if number == total_sum:
            numbers.append(number)
    return numbers

def armstrong(n):
    numbers = []
    order = len(str(n))
    for k in range(1, order):
        a = [i ** k for i in range(10)]
        for b in combinations_with_replacement(range(10), k):
            x = sum(map(lambda y: a[y], b))
            if x > 0 and tuple(int(d) for d in sorted(str(x))) == b:
                numbers.append(x)
    return sorted(numbers)

# Correctness
assert original(1000) == armstrong(1000)

# Benchmark
n = 1_000_000

initial_time = time.perf_counter()
original(n)
total_time = time.perf_counter() - initial_time
print(f'Original: {round(total_time, 3)} s')

initial_time = time.perf_counter()
armstrong(n)
total_time = time.perf_counter() - initial_time
print(f'Armstrong: {round(total_time, 3)} s')


Program for activity selection exceeds time limit I've been trying to solve this problem for some while and the solution I have come up with exceeds the lime limit by 1-3 ms and 2 out of 10 tests get a memory issue error. Why is that? This is the code for my solution:

#include <fstream>
#include <vector>
#include <algorithm>
#include <iostream>

using namespace std;

ifstream in("planificare.in");
ofstream out("planificare.out");

struct show {
    int start, finish, used = 0;
};

bool end_time (show lhs, show rhs) {
    if (lhs.finish == rhs.finish) return lhs.start < rhs.start;
    return lhs.finish < rhs.finish;
}

int main() {
    int participants, tv_chan;
    vector<show> prog;
    in >> participants >> tv_chan;
    for (int i = 1; i <= participants; i++) {
        show a;
        in >> a.start >> a.finish;
        prog.push_back(a);
    }
    sort(prog.begin(), prog.end(), end_time);
    int maxActivities = 0;
    int unused = 0;
    for (int i = 1; i <= tv_chan; i++) {
        while (prog[unused].used)
            unused++;
        int lastFinish = prog[unused].finish;
        prog[unused].used = 1;
        maxActivities ++;
        for (int j = 0; j < prog.size(); j++) {
            if (prog[j].start >= lastFinish && prog[j].used == 0) {
                maxActivities++;
                lastFinish = prog[j].finish;
                prog[j].used = 1;
            }
        }
    }
    out << maxActivities;
}

Planificare - InfoArena

  Talent Day is coming soon at the Blomkvist TV channel and the CEO Mike
  needs you to make the program grid. \$N\$ participants have enrolled
  to expose their talents, each communicating the amount of time it
  needs. Mike's TV Chain consists of \$K\$ stations, (Blomkvist 1, Blomkvist 2,
  ... Blomkvist K) that transmit independently of each other. Due to the
  fact that all \$K\$ stations are all just as popular as the other, the participants are indifferent
  to which they will appear.

  Knowing that at any given moment any station
  will broadcast a single show, determining the maximum number of shows
  that can be televised.

  Input data

  The input file planificare.in will contain on the first line \$2\$ natural numbers: \$N\$
  and \$K\$. On each of the following \$N\$ lines there will be 2 values,
  the starting time and the end time, representing the time interval during which the
  participant performs its activity.

  Output data

  The output file planificare.out will contain the number requested by Mike on
  the first line.

  Restrictions and clarifications

  \$1 \le N \le 100,000\$

  \$1 \le K \le 100,000\$

  \$1 \le \text{start}_i \le \text{stop}_i \le 1,000,000,000\$

  Time limit: 0.2 seconds

  For 30% of the tests, \$N\$ ≤ 2000 and for another 10% of the tests, \$K\$ = 1.
     At each channel, a show can start at the same time that the previous one is over.

  Example

  planificare.in

2 1
1 4
4 8

  planificare.out

2

The problem is that you are erasing elements from prog and then without bounds checking access it with prog[j]. That is sure to fall over.

Your end_time function is commonly known as operator< So you should use the appropriate name.

There is no comment on the significance of 1 and why a program should not be reordered when it starts at 1. It seems that different programs with starting time 1 should still have an ordering.

You are inconsistent in your loops. Once goes from 1 to <= participants the other goes from 0 to < participants. The latter is the commonly used one.

The first loop seems to sort for start time? You can achieve the same if you create a proper operator<.

using namespace std; is bad practice. There is no benefit to it other that you do not really know what is standard and what not, which is actually bad. Also it isprone to name clashes. Use what you need and not everything C++ has in the box


Multithreading decouple update-logic and render-logic I want to improve my multithreading competence and tried to build a compact doublebuffered way to separate two threads where one is only displaying the state and the other one updates the state. So I am here to hear whether I done it "right" and what to improve.

#include <thread>
#include <mutex>
#include <iostream>
#include <fstream>

template <class T> class StateBuffer
{
public:
    typedef T value_t;

    void Swap()
    {
        {
            std::lock_guard<std::mutex> _(swapMutex);
            index = 1 - index;
        }
        Write() = Read();
    }
    value_t& Write()
    {
        std::lock_guard<std::mutex> _(swapMutex);
        return states[index];
    }
    const value_t Read()
    {
        std::lock_guard<std::mutex> _(swapMutex);
        return states[1-index];
    }
    const value_t&& ReadRef()
    {
        std::lock_guard<std::mutex> _(swapMutex);
        return std::ref(states[1-index]);
    }
private:
    mutable std::mutex swapMutex;
    T states[2];
    uint8_t index = 0; // index = write, 1-index = read
};

struct state_t
{
    int count = 0;
    bool stop = false;
};
typedef StateBuffer<state_t> TestStateBuffer;
TestStateBuffer state;

auto constexpr NO_TIME = std::chrono::seconds(0);

void r()
{
    std::ofstream log("log.txt");
    bool run = true;
    while (run)
    {
        state_t display = state.Read();

        log << display.count << "\n";
        run = !display.stop;

        std::this_thread::sleep_for(NO_TIME);
    }
    log.flush();
}

void u()
{
    bool run = true;

    while (run)
    {
        state_t& write = state.Write();

        run = !write.stop;
        write.stop = ++write.count > 1000000;

        state.Swap();
        std::this_thread::sleep_for(NO_TIME);
    }
}

int main(int, char**)
{
    std::thread render(r);
    std::thread update(u);

    update.join();
    render.join();

    std::cout << "ende" << std::endl;
    std::cin.get();

    return 0;
}

Yes, this is intended to find use in gamedev.

Questions that have gone through my mind:

I use sleep(0) to force a thread swap. Do I need this?
Is there a way how to solve this with a conditional_variable as I read that locking a mutex is expensive in comparison to check with a conditional_variable (although one still need a mutex)?
As I am using references is this really safe?
Have I left anything out?

Environment: Windows 7 x64 g++ on MinGW / Mint 17 x64 clang++ (both C++11 enabled)

No. Using sleep is the wrong way in any most cases that you can think of. Use the correct functions for this. The C++ standard does not contain a standardized way of doing so. Please use SwitchToThread for Windows and pthread_yield for Unix. To gain more information, you may ask a search engine and for context switching, this is a more common identifier than swapping.
Yes, you are right. Mutual exclusion is indeed slower than a condition variable, whereas it is not always replaceable. Since I don't know how your compiler implements mutexes, you may want to search for differences between mutex and critical sections on Windows. I don't know if there are two different ways also on Unix and other systems. But to answer your question: Yes, one can solve this with condition variables. The difference is very-well stated here.
References are as safe as pointers, since all references are pointers internally after the compiler has done his work. Just be sure not to pass references, that you are expecting to own a resource exclusively, to functions which don't own the resource at all. This leads to very unpredictable code, as wrong coding in multi threaded environments always does.
Seems to be okay. But don't count on my word in the case of the move-operator. I simply don't know if this is going to be valid.


Angle reflection function I've developed a function to calculate the exit angle of a projectile after it collides with a boundary/edge inside a rectangular space, based on the entry angle and the axis it is colliding with.

Given those two factors, the actual boundary is implied, since a projectile travelling at an angle of 315° (where 0°/360° sit at 3 o'clock) and colliding with a horizontal boundary (the x axis) must be colliding with the top boundary.

The current implementation came about through iterative refactoring of a much more verbose and redundant version, however I can't vocalise exactly why some parts are necessary, but they appear to satisfy the requirements and I'm confident it works based on a number of tests.

function reflectAngle(angle, axis) {

    if (angle % 90 === 0)
        return (angle + 180) % 360;

    let reflected;
    let segment = (angle % 90) * 2;

    if (axis === 'X')
        reflected = angle + (180 - segment);

    else if (axis === 'Y')
        reflected = angle - segment;

    if (angle < 90)
        reflected += 180;

    return reflected % 360;
}

Can this function be simplified or improved for readability?

Review

A little ambiguous, and overly complex. You need to familiarize your self with radians and forget about degrees

Style

Block-less blocks

A block is a section of code delimited by {...} many C like syntax languages allow you to skip the block delimiters for single line blocks after conditional statements.

This can be a major source of life long frustration, and there is not a programmer alive that does not know the insidious nature of the bugs that can result when you make changes to the code and forget the {...}

// worst
if (foo === bar)
       foo = poo

// bad
if (foo === bar)
       foo = poo;

// better but be consistent never mix this with above
if (foo === bar) foo = poo;

// best
if (foo === bar) { foo = poo } // note that ; and } denote the end of
                                // an expression, you don't need to use both

// or
if (foo === bar) {
   foo = poo;
}

Constants

So many magic numbers. If you find your self adding the same numbers over and over it is a good sign that a defined constant is better. You never know when things may change. I.E. switch units from degrees to radians.

const A90  = 90;
const A180 = 180;
const A360 = 360;

Then if you want to change to radians

const A90  = Math.PI / 2;
const A180 = Math.PI;
const A360 = Math.PI * 2;

or clock angles

const A90  = 3;
const A180 = 6;
const A360 = 12;

No need to find each number in the code base.

Logic

The axis argument is ambiguous. Axis? is it the axis of reflection or the axis to reflex from. Also you use a string and to humans "x", "X" and "y", "Y" have the same meaning, but your code sees them differently, this is never a good thing. It would pay to check both (upper and lower) or convert to lowercase, or best use a defined constant,
Why are you using degrees, NO Math functions uses degrees, the only time you need to use degrees is for output, which generally is never needed. Learn to work in radians, it makes a lot of angle related maths so much easier.
The code is just too complex for something so simple. The problem is just one of negating the scalar associated with the direction of reflection. I.E. a vector has two scalars x,y if you reflect along X you negate x, if you reflex along Y you negate y.

Messing around with the cyclic angle is not needed. I have a general rule of thumb that if I ever find my self having to normalize a cyclic value E.G. (angle
% 360) I am doing it wrong and there is a better way.

Rewrite

This is how I would write the function. I would never use degrees, that is only ever needed for display only. That makes the function much simplier.

// defined axis names
const AXIS = {x : 0, y : 1}

// function computes the reflected angle
// rad Incoming angle in radians
// dir direction of reflection. Any of AXIS, defaults to AXIS.y
function reflectAngle(rad, dir) {
    return dir === AXIS.x ? Math.acos(-Math.cos(rad)) : Math.asin(-Math.sin(rad));
}

var reflected = reflectAngle(1, AXIS.x);

Update:

Sorry totally did not add the correct example!

As explained in point 3 (above) mirror x or y depending on which axis you are reflecting from.

function reflectAngle(rad, dir) {
    const c = Math.cos(rad), s = Math.sin(rad);
    return Math.atan2(...(dir === "X" ? [s, -c] : [-s, c]));
}


Using Array.prototype.filter() and Array.prototype.map() I need to first filter items for an array of objects {type: 'genre', value: '123'} and after combine in a string content for property value only.

Any suggestions on how to improve the following code?

this.flattenCategories = function () {
    // flatten in a single string all categories so to be easily visaulised in the dom
    var categories = this.information.filter(function (item) {
        if (item.type === 'genre') {
            return item;
        }
    });
    this.categoriesFlatten = categories.map(function (item) {
        return item.value;
    }).join(CONFIG.app.categoriesSeparator)
},

You have a bug in your code. Filter expects true or false. Your still works because implicit type conversion of Javascript (object => true, undefined => false).

Correct would be:

var categories = this.information.filter(function (item) {
    return item.type === 'genre';
});

Filter/Map could be combined together if you like.

You're using variable from outside. You could improve testability and readability by using of arguments and return the result.

this.flattenCategories = function (information, separator) {
    // flatten in a single string all categories so to be easily visaulised in the dom
    return information
        .filter(function (item) { return item.type === 'genre'; })
        .map(function (item) { return item.value; })
        .join(separator);
}


Managing many availability status I'm a junior who wants to refactor a big method in my code, which has been tested and works:

  def self.authorized_status plugin_id, store_id, application_id, version, locale=nil
    if plugin_id.match /^(\w|-)+$/
      I18n.locale = locale || :en # I think I can move it to a concern

      # for this I thought about an external method how return two values, or a hash. Any idea?
      found_by = :iid
      plugin = Plugin.find_by_iid(plugin_id)
      if !plugin
        plugin = Plugin.find_by_aid(plugin_id)
        found_by = :aid
      end

      application_id_and_version = { application_id: application_id, version: version, status: Application::PUBLISHED_STATUS }

      update_by_application_id = ApplicationUpdate.where(application_id: application_id).joins(application: :store).where(application: {store_id: store_id} )
      is_store = update_by_application_id.first.application.is_store? if !update_by_application_id.first.nil?

      # for the rest of the code I don't know what to do expect put validations into separate methods.
      if plugin.nil? || plugin.user.nil?
        { status: UNREGISTERED_PLUGIN, message: I18n.t('....') }
      elsif ApplicationUpdate.where(application_id_and_version).count == 0 && !is_store
        { status: UNKNOWN_APPLICATION, message: I18n.t('....') }
      else
        if !is_store
          groups = Group.joins(users: :plugins).where(users: {plugins: {found_by => plugin_id}})
          .joins(:application_updates)
          .where(application_updates: application_id_and_version)
        end

        if !is_store && groups.empty?
          { status: NOT_AUTHORIZED, message: I18n.t('....') }
        else
          store = Store.find store_id
          authorized = (!store.preauthorized?) || is_store || plugin.authorized_for_store?(store)
          if !authorized || BlacklistedPlugin.find_by_organisation_id_and_plugin_id(store.organisation_id, plugin.id)
            { status: NOT_AUTHORIZED, message: I18n.t('....') }
          else
            { status: AUTHORIZED, message: I18n.t('....') }
          end
        end
      end
    else
      { status: PLUGIN_ID_FORMAT_ERROR, message: I18n.t('....') }
    end
  end

I saw recently to get some help a talk from Andy Pike (slide) with some nice principle like OCP or SOLID. I don't think the way he refactors the code is something I should do but maybe I'm wrong. To be honest, the method sometimes looks so weird, especially with all the ifs, that I don't know where to start.

This method feels like it should be its own class, and the arguments to the method should be constructor arguments. This is really where a "service" class can clean things up, because this method is really doing a lot.

First, I'd like to draw attention to the following lines:

   def self.authorized_status plugin_id, store_id, application_id, version, locale=nil
     if plugin_id.match /^(\w|-)+$/
       I18n.locale = locale || :en # I think I can move it to a concern

The "locale" is being set as a class-level value, which I understand is needed later on. This might have the unintended side affect of changing the locale for outside code. Consider this:

# Change locale to Spanish
I18n.locale = :es

status = YourClass.authorized_status(plugin_id, store_id, application_id, version)

# I18n.locale is now :en

You've now changed the locale for the entire application just by calling this method. In the very least you should save the "current locale" to a local variable, and at the end just before returning the status hash set the locale back to what it was before.

Aside from that, let the refactoring begin!

Given that the return value is a Hash, I'm assuming this is returned from some API controller that converts the Hash to JSON. You have this Business Logic of detecting authorized plugins, which you then couple to the output format of your application back to the client. I would create a class specifically to hold the plugin authorization status:

class PluginAuthorization
  attr_reader :plugin, :status, :message

  def initialize(plugin, is_authorized, status, message)
    @plugin = plugin
    @is_authorized = is_authorized
    @status = status
    @message = message
  end

  def authorized?
    @is_authorized
  end

  def to_h
    { status: status, message: message }
  end
end

This has three advantages:

You are not coupling your Business Logic to your controller's output format
You have a concrete class you could repurpose for a web page as well as a JSON return type
You have a clearly defined interface for just what the authorization data looks like.

Now we can start building our plugin authorization service class -- in fact, that's a perfect name for this class:

class PluginAuthorizationService
  def initialize(plugin_id, store_id, application_id, version)
    # Initialize instance variables
  end

  def authorize(locale = :en)
    # Create and return a new PluginAuthorization object
  end
end

This gives us the basic skeleton to start our refactoring job. Without diving into the gritty details, let's look how you would use this service class in an API controller:

class PluginsController < ActionController::API
  def authorize
    plugin_id = params[:plugin_id]
    store_id = params[:store_id]
    application_id = params[:application_id]
    version = params[:version]
    locale = params[:locale]
    authorization_service = PluginAuthorizationService.new plugin_id, store_id, application_id, version
    authorization = authorization_service.authorize locale

    if authorization.authorized?
      # do something
    else
      # do something else
    end

    render json: authorization.to_h
  end
end

This keeps your controller simple, plus the business logic of authorizing a plugin is encapsulated by the service class. Now that we have an idea what our end goal will be, let's fill in the blanks (and they are big blanks).

First, the status constants should go in the service class, plus let's flesh out the constructor so it does as little work as possible (like any constructor should).

class PluginAuthorizationService

  PLUGIN_ID_FORMAT_ERROR = '...'
  UNREGISTERED_PLUGIN = '...'
  UNKNOWN_APPLICATION = '...'
  NOT_AUTHORIZED = '...'
  AUTHORIZED = '...'

  def initialize(plugin_id, store_id, application_id, version)
    @plugin_id = plugin_id
    @store_id = store_id
    @application_id = application_id
    @version = version
    @application_id_and_version = {
      application_id: application_id,
      version: version,
      status: Application::PUBLISHED_STATUS
    }
  end

  # ...
end

We need a plugin Id, store Id, application Id and version. The original code had the locale default to :en so we can supply that as an argument to the authorize method. The only other thing we do is create a Hash for the @application_id_and_version since this is referenced later as part of the WHERE condition for some queries.

The first bit of cleaning up is the plugin variable, which can be found using one of two methods, with the additional requirement that the method used to find the plugin is used elsewhere in a query:

class PluginAuthorizationService
  # ...

private

  def plugin
    return @plugin unless @plugin.nil?

    @plugin = Plugin.find_by_iid @plugin_id

    if @plugin.nil?
      @plugin_found_by = :aid
      @plugin = Plugin.find_by_aid @plugin_id
    else
      @plugin_found_by = :iid
    end

    @plugin
  end

  def plugin_found_by
    plugin
    @plugin_found_by
  end

Any time you need a Plugin object you call the plugin method. The plugin_found_by method calls plugin to make sure the method by which the plugin is found gets calculated.

Next, we have some ActiveRecord queries to clean up and encapsulate into their own methods:

   # Query 1
   ApplicationUpdate.where(application_id: application_id).joins(application: :store).where(application: {store_id: store_id} )

   # Query 2
   ApplicationUpdate.where(application_id_and_version).count == 0

   # Query 3
   Group.joins(users: :plugins).where(users: {plugins: {found_by => plugin_id}})
     .joins(:application_updates)
     .where(application_updates: application_id_and_version)

We will create methods on the service class for these queries:

private

  # Query 1
  def application_update
    @application_update ||= ApplicationUpdate.where(application_id: @application_id)
                                             .joins(application: :store)
                                             .where(application: {store_id: @store_id} )
                                             .first
  end

  # Query 2
  def application_update_exists?
    ApplicationUpdate.where(@application_id_and_version).count > 0
  end

  # Query 3
  def plugin_groups
    @plugin_groups ||= Group.joins(users: :plugins)
                            .where(users: {plugins: {plugin_found_by => @plugin_id}})
                            .joins(:application_updates)
                            .where(application_updates: @application_id_and_version)
  end

Notice that the plugin_groups method calls the aforementioned plugin_found_by method in the WHERE clause for the query.

Next, let's focus on the following line of code in the original post:

   is_store = update_by_application_id.first.application.is_store? if !update_by_application_id.first.nil?

This too is a good candidate for its own method, but first let's modify the ApplicationUpdate class to make it easier to adhere to the Principle of Least Knowledge:

class ApplicationUpdate < ActiveRecord::Base
  delegate :is_store?, :to => :application, :allow_nil => true

  # ...
end

This allows us to call application_update.is_store? and not require outside code to traverse the application property. Now, the method on PluginAuthorizationService:

private
  # ...

  def is_store?
    application_update.try :is_store?
  end

Calling application_update.try removes the need for a nil-check.

Another opportunity for properly encapsulated code is in the following line:

   if !authorized || BlacklistedPlugin.find_by_organisation_id_and_plugin_id(store.organisation_id, plugin.id)

The Plugin class should have a method that performs this logic:

class Plugin < ActiveRecord::Base
  # ...

  def blacklisted?(organization_id)
    !!BlacklistedPlugin.find_by_organisation_id_and_plugin_id(organisation_id, self.id)
  end
end

We will also add a few more helper methods to our service class to make our lives easier:

private
  # ...

  def plugin_unregistered?
    plugin.nil? || plugin.user.nil?
  end

  def store
    @store ||= Store.find @store_id
  end

  def valid_plugin_id?
    @plugin_id.match /^(\w|-)+$/
  end

  def authorized?
    @is_authorized ||= !store.preauthorized? || is_store? || plugin.authorized_for_store?(store)
  end

end # End of PluginAuthorizationService class

We've only seen bits and pieces of this, so let's put things all together, and see how the authorize method on the service class ends up:

class PluginAuthorizationService

  PLUGIN_ID_FORMAT_ERROR = '...'
  UNREGISTERED_PLUGIN = '...'
  UNKNOWN_APPLICATION = '...'
  NOT_AUTHORIZED = '...'
  AUTHORIZED = '...'

  def initialize(plugin_id, store_id, application_id, version)
    @plugin_id = plugin_id
    @store_id = store_id
    @application_id = application_id
    @version = version
    @application_id_and_version = {
      application_id: application_id,
      version: version,
      status: Application::PUBLISHED_STATUS
    }
  end

  def authorize(locale = :en)
    previous_locale = I18n.locale
    I18n.locale = locale

    authorization = if !valid_plugin_id?
                      PluginAuthorization.new plugin, false, PLUGIN_ID_FORMAT_ERROR, I18n.t('....')
                    elsif plugin_unregistered?
                      PluginAuthorization.new plugin, false, UNREGISTERED_PLUGIN, I18n.t('....')
                    elsif !application_update_exists? && !is_store?
                      PluginAuthorization.new plugin, false, UNKNOWN_APPLICATION, I18n.t('....')
                    elsif !is_store? && plugin_groups.empty?
                      PluginAuthorization.new plugin, false, NOT_AUTHORIZED, I18n.t('....')
                    elsif !authorized? || plugin.blacklisted?(store.organisation_id)
                      PluginAuthorization.new plugin, false, NOT_AUTHORIZED, I18n.t('....')
                    else
                      PluginAuthorization.new plugin, true, AUTHORIZED, I18n.t('....')
                    end

    I18n.locale = previous_locale

    authorization
  end

private

  def application_update
    @application_update ||= ApplicationUpdate.where(application_id: @application_id)
                                             .joins(application: :store)
                                             .where(application: {store_id: @store_id} )
                                             .first
  end

  def application_update_exists?
    ApplicationUpdate.where(@application_id_and_version).count > 0
  end

  def authorized?
    @is_authorized ||= !store.preauthorized? || is_store? || plugin.authorized_for_store?(store)
  end

  def plugin_groups
    @plugin_groups ||= Group.joins(users: :plugins)
                            .where(users: {plugins: {plugin_found_by => @plugin_id}})
                            .joins(:application_updates)
                            .where(application_updates: @application_id_and_version)
  end

  def is_store?
    application_update.try :is_store?
  end

  def plugin
    return @plugin unless @plugin.nil?

    @plugin = Plugin.find_by_iid @plugin_id

    if @plugin.nil?
      @plugin_found_by = :aid
      @plugin = Plugin.find_by_aid @plugin_id
    else
      @plugin_found_by = :iid
    end

    @plugin
  end

  def plugin_found_by
    plugin
    @plugin_found_by
  end

  def plugin_unregistered?
    plugin.nil? || plugin.user.nil?
  end

  def store
    @store ||= Store.find @store_id
  end

  def valid_plugin_id?
    @plugin_id.match /^(\w|-)+$/
  end

end

The authorize method has been reduced to 22 lines, and by utilizing properly named methods, authorize reads more like natural language and becomes much easier to understand:

if not a valid plugin id
    ...
else if plugin unregistered?
    ...
else if not application_update_exists? and not is_store?
    ...
else if not is_store? and plugin_groups is empty?
    ...
else if not authorized? or plugin blacklisted? by store.organisation_id
    ...
else
    Authorized!
end


Merge two sorted arrays, make more efficent (O(a+b)) Problem :Given two sorted arrays, merge them to form a new array of unique items.

  If an item is present in both arrays, it should be part of the resulting array if and only if it appears in both arrays the same number of times.

  e.g.: a = [1, 3, 40, 40, 50, 60, 60, 60] and b = [2, 40, 40, 50, 50, 65],
    should return [1, 2, 3, 40, 60, 65]

  a = [2,2,2,2,40,40,50] and b = [40,40,60] should return [2,40,50,60]

  a = [5,5] and b = [5,5,5] should return []

My problem is that my function is not efficient enough (runtime takes too long), is it possible to may it more efficient? Possibly O(a+b)

  function mergeArrays(a, b) {

      function countInArray(array, array2, what) {
          var count = 0;
          var count2 = 0;

          for (var i = 0; i < array2.length; i++) {
              if (array2[i] === what) {
                  count2++;
              }
          }
          for (var i = 0; i < array.length; i++) {
              if (array[i] === what) {
                  count++;
              }
          }
          return count === count2;
      }

      var arr = []
      for (x = 0; x < b.length; x++) {
          if (!a.includes(b[x]) && !arr.includes(b[x]))
              arr.push(b[x])
      }

      for (i = 0; i < a.length; i++) {
          if (countInArray(a, b, a[i]) && !arr.includes(a[i])) {
              arr.push(a[i])
          } else {
              if (!arr.includes(a[i]) && !b.includes(a[i]))
                  arr.push(a[i])
          }
      }

      console.log(arr)
      return arr.sort(function(a, b) {
          return a - b
      });
  }

Current code

The time complexity of your current code is indeed quite high and can be made \$O(a+b)\$ as you suggest.

But first, a few remarks about the code (some of these were already covered by @Tushar while I was writing this answer):

Nested functions: It's not very efficient to define a function inside a function. As far as I know, most JS interpreters would actually construct a new closure (simply – a new function) each time mergeArrays gets called. There's no harm in defining countInArrays next to mergeArrays, instead of inside it, and it will probably make your code faster.
Scope of variables:

The for-loop variables x and i in function mergeArrays are not declared with the var or let keyword, which makes them global. That means they will exist even after your function returns. For example, you could call your function, and after it returns you could print the value of x, because it's in global scope and therefore accessible from everywhere. You probably don't want that, so it's better to use the var keyword, as you do in the countInArray function.
However, a variable declared in a function using the var keyword has function scope. That means, if you use i in the first for and also in the second for, then the second var i = 0 is in fact a re-declaration of an already-existing variable. Fortunately ECMAScript 6 introduced the let keyword, which declares a variable within the scope of the block. That means a variable declared as for (let i = … will "cease to exist" after that for loop.
Ergo, if using i just within the for-loop, it's preferable to do it this way:

for (let i = 0; i < a.length; i++) …

Optimized version

@Tushar has posted a more performant version. However, if I'm not mistaken, the asymptotic time complexity of his improved version is the same as – or very similar to – the complexity of your original version.

Here is an implementation with \$O(a+b)\$ asymptotic time complexity.

A few things to note:

there is some code duplicity and it surely could be written more nicely,
I have not tested it very thoroughly – only on the samples you provided in your question,
EDIT: performance benchmark.

function mergeArrays2(a, b) {

    let res = [];
    let wasInBoth;  // Indicates whether a repeating value
                    // was in both `a` and `b`.

    let i = 0;
    let j = 0;

    while (i < a.length && j < b.length) {
        const elemA = a[i];
        const elemB = b[j];

        if (elemA === elemB) {
            // Same elements – add them both if not already present.
            if (!(elemA === res[res.length - 1] && wasInBoth)) {
                res[res.length] = elemA;
                wasInBoth = true;
            }
            i++;
            j++;
            continue;
        }

        // Different elements – check if either of them is already
        // in `res` and was in both `a` and `b`. If yes, remove it
        // from `res`, because its number of occurrences in `a` and `b`
        // is not the same.
        if (wasInBoth) {
            wasInBoth = false;  // Not any more.

            if (elemA === res[res.length - 1]) {
                // `a` contains more occurrences of the element than `b`.
                // Remove the element from `res` and skip
                // all the occurrences of the element in `a`.
                res.length--;
                while (a[++i] === elemA);
                continue;
            }
            else if (elemB === res[res.length - 1]) {
                // `b` contains more occurrences of the element than `a`.
                // Remove the element from `res` and skip
                // all the occurrences of the element in `b`.
                res.length--;
                while (b[++j] === elemB);
                continue;
            }
        }

        // Different elements – select the least.
        // Add it if not already present.
        if (elemA < elemB) {
            if (elemA !== res[res.length - 1]) {
                res[res.length] = elemA;
            }
            i++;
        }
        else {
            if (elemB !== res[res.length - 1]) {
                res[res.length] = elemB;
            }
            j++;
        }
    }

    // Process the rest of `a` or `b`, whichever is not done.
    let [rest, idx]  = (i < a.length)? [a, i] : [b, j];

    while (idx < rest.length) {
        const elem = rest[idx];

        if (wasInBoth) {
            wasInBoth = false;

            if (elem === res[res.length - 1]) {
               // Same as before – remove from `res`
               // and skip all further occurrences.
                res.length--;
                while (rest[++idx] === elem);
                continue;
            }
        }

        // Same as before – add if not present.
        if (elem !== res[res.length - 1]) {
            res[res.length] = elem;
        }
        idx++;
    }

    return res;
}


Open encrypted PDF file by trying passwords from a word list As an exercise, I wrote a program to decrypt a PDF file.  The only thing that is known about the encryption password is that it is a single English word (all capital or lowercase).

The program uses a dictionary file containing over 45000 words to unlock the file. It looks like this:

dictionary.txt

AARHUS
AARON
ABABA
ABACK
ABAFT
ABANDON
ABANDONED
ABANDONING
ABANDONMENT
ABANDONS
ABASE
ABASED
ABASEMENT
ABASEMENTS
ABASES
ABASH
ABASHED
ABASHES
⋮
ZEROTH
ZEST
ZEUS
ZIEGFELD
ZIEGFELDS
ZIEGLER
ZIGGY
ZIGZAG
ZILLIONS
ZIMMERMAN
ZINC
ZION
ZIONISM
ZIONIST
ZIONISTS
ZIONS
ZODIAC
ZOE
ZOMBA
ZONAL
ZONALLY
ZONE
ZONED
ZONES
ZONING
ZOO
ZOOLOGICAL
ZOOLOGICALLY
ZOOM
ZOOMS
ZOOS
ZORN
ZOROASTER
ZOROASTRIAN
ZULU
ZULUS
ZURICH

pdf_password_breaker

"""
Brute force password breaker using a dictionary containing
English words.
"""

import sys
import PyPDF2
from pathlib import Path

def get_filename_from_user() -> Path:
    """Asks for a path from the User"""
    while True:
        filename: str = input("Enter filename in folder of script:")
        path: Path = Path(sys.path[0], filename)

        if path.is_file():
            return path.as_posix()
        print("File doesn't exist\n")

def decrypt(pdf_filename: Path, password: str) -> bool:
    """
    Try to decrypt a file. If not successful a false is returned.
    If the file passed is not encrypted also a false is passed
    """
    with open(pdf_filename, 'rb') as pdf_file:
        pdf_reader = PyPDF2.PdfFileReader(pdf_file)
        pdf_reader.decrypt(password)
        pdf_writer = PyPDF2.PdfFileWriter()

        try:
            for page_number in range(pdf_reader.numPages):
                pdf_writer.addPage(pdf_reader.getPage(page_number))
        except PyPDF2.utils.PdfReadError:
            return False

        new_name: str = pdf_filename.stem + "_decrypted.pdf"
        filename_decrypted = pdf_filename.parent / new_name

        with open(filename_decrypted, 'wb') as pdf_file_decrypted:
            pdf_writer.write(pdf_file_decrypted)
    return True

def break_encryption(pdf_filename: Path, dictionary_filename: str) -> bool:
    """Try's out words from a dictionary to break encryption"""
    with open(dictionary_filename, 'r') as dictionary_file:
        keyword: str = dictionary_file.readline().strip()

        if decrypt(pdf_filename, keyword):
            return keyword
        if decrypt(pdf_filename, keyword.lower()):
            return keyword.lower()

        while keyword:
            keyword = dictionary_file.readline().strip()

            if decrypt(pdf_filename, keyword):
                return keyword
            if decrypt(pdf_filename, keyword.lower()):
                return keyword.lower()
    return None

def pdf_password_breaker():
    """main loop"""
    filename: Path = get_filename_from_user()
    password: str = break_encryption(filename, "dictionary.txt")

    if password:
        print("File unlocked. Password was:" + password)
        return
    print("File could not be unlocked")

if __name__ == "__main__":
    pdf_password_breaker()

I tried a file with a simple password like "hello". It works but it takes a lot of time until it reaches the "hello" in the file.

I wonder if theres a way to improve the speed, or any other aspect of the code.

Looking at the documentation of PdfFileReader.decrypt It states that it return 0 if the file fails or 1 if it succeeds.

You can also use isEncrypted to check if the file was encrypted in the first place.

So you can turn the loop around. First open the pdf file see if it was encrypted at all and if so try to call decrypt with every password in your dictionary.


Is there a better way to assign constructor parameter in Typescript? I have a class with a various set of optional parameters when instantiated.
I would like to be sure that the user input ONLY the available parameters option, and also add a default value if nothing is passed.

I come with this

export enum HexagoneType {
  POINTY,
  FLAT
}

export interface HexagoneOptions {
  center?: Vector2;
  size?: number;
  color?: string;
  type?: HexagoneType;
}

export default class Hexagone {
  ctx: CanvasRenderingContext2D;
  center: Vector2;
  size: number;
  color: string;
  type: HexagoneType;

  constructor(ctx: CanvasRenderingContext2D, options?: HexagoneOptions) {
    this.ctx = ctx;
    this.center = (options && options.center) ? options.center : new Vector2();
    this.size = (options && options.size) ? options.size : 100;
    this.color = (options && options.color) ? options.color : "red";
    this.type = (options && options.type) ? options.type : HexagoneType.POINTY;
  }
}

But is there something shorter ?

This is typical example of service locator anti pattern that should be refactored to builder pattern.

export class Hexagone {
  constructor(
    private ctx: CanvasRenderingContext2D,
    private center: Vector2,
    private size: number,
    private color: string,
    private type: HexagoneType
  ) {}
}

export class HexagoneBuilder {
  public center?: Vector2;
  public size?: number;
  public color?: string;
  public type?: HexagoneType;

  constructor(private ctx: CanvasRenderingContext2D) {}

  create() {
    return new Hexagone(
      this.ctx,
      this.center ?: new Vector2(),
      this.size ?: 100,
      this.color ?: "red",
      this.type ?: HexagonType.POINTY
    )
  }
}

Sorry if i made some syntax errors, I dont work with ts on daily basis, but you should get the idea...


A singly linked list implementation in C How is this Implementation of a list in C?

Please tell me whether it is good or please mention any problems.
It is a singly linked List and I tried to do it recursively.

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

struct node{
    int value;
    struct node *next;
};

void add(struct node *lst, int num){
    if(lst->next == NULL){
        lst->value = num;
        lst->next = (struct node*)malloc(sizeof(struct node*));
        lst->next->next = NULL;
    }else{
        add(lst->next, num);
    }
}

void print_lst(struct node *lst){
    if(lst->next == NULL){

    }else{
        printf("\n%d", lst->value);
        print_lst(lst->next);
    }
}

int length(struct node *lst){
    if(lst->next == NULL){
        return 0;
    }else{
        return 1 + length(lst->next);
    }
}

int get(struct node *lst, int pos){
    if(pos < 0 ||  pos >= length(lst)){
        fprintf(stderr ,"\nIndexOutOfBoundsException\n");
    }else if(pos > 0 && pos <length(lst)){
        pos += -1;
        get(lst->next, pos);
    } else if (pos == 0){
        return  lst->value;
    }
}

int main ( ) {

    struct node lst;
    lst.next = NULL;
    add(&lst, 13);
    add(&lst, 12);
    add(&lst,  1);
    add(&lst, 10);
    add(&lst, 10);
    add(&lst, 4);
    //print_lst(&lst);
    printf("\n%d", get(&lst, 2));
    printf("\n%d", get(&lst, 5));
    printf("\n%d", get(&lst, 13));

    printf("\n\n%d", length(&lst));
    return 0;
}

Do not cast the result of malloc. If your code does #include <stdlib.h>, the cast is redundant. If it does not, the cast just masks the warning, which may lead to hard to find bugs.
Prefer sizeof(object) to sizeof(type). The latter leads to the double maintenance problem, in case the type is changed.
I strongly recommend to have a constructor-like function to create a node:

struct node * create_node(int num)
{
    struct node * node = malloc(sizeof(*node));
    node->value = num;
    node->next = NULL;
    return node;
}

It will spare you plenty of trouble when the definition of struct node changes.
You should get a warning that get doesn't always return a value (if you do not, enable all warnings, or change the compiler). It is a very serious warning, and in real life the code with such a problem leads to some dire consequences. You must return get(lst->next, pos); in a recursive clause.

The fact that your main printed expected values is an unlucky coincidence.

What to return in the IndexOutOfBound situation is a different matter. Consider returning an error code along with the value.
Calling length(lst) at each level of recursion degrades performance to quadratic.
As a side note, rather than testing for position < 0, consider passing a position as some unsigned type. size_t is the most obvious candidate.
Along the same line, a low level utility function such as get should refrain from printing anything.
I strongly advise agains recursion when an iterative solution is immediately available.


Best way to retrieve parameters from associative array in javascript? Within a function, I want to grab a set of parameters from a named array. What is the most efficient way of testing if the parameter exists in the array, and if it does, use it as a variable? I came up with a few alternatives.

target = $('#page_links')
if ( functionobject.target ) {
    target = functionobject.target
}

//method 2:
var target = ''
if ( functionobject.target ) {
    target = functionobject.target
}
else {
    target = final_array
}

//method 3:
target = functionobject.target
if (typeof target == 'undefined') {
    target = $('#page_links')
}

I would say

var target = functionobject.target || $('#page_links');

How did I arrive at that?  The important point to get across is that no matter what happens, you want to assign something to target.  One first attempt to express that would be to use the ternary operator

var target = functionobject.target ? functionobject.target : $('#page_links');

However, the repetition of functionobject.target is ugly.  It would be better to use the short-circuit evaluation feature of the a || b operator.  If a is true, then evaluation stops, and the value of the expression is just a.  Otherwise, the value of the expression is b.


KMP string match algorithm in Python Write the implementation for generating pattern part, and did some testing, seems good to me. But not sure if any functional bugs? If anyone find a functional bug, appreciate for the insights.

def findPattern(pattern):

    j = -1
    next = [-1] * len(pattern)
    i = 0 # next[0] is always -1, by KMP definition

    while (i+1 < len(pattern)):
        if (j == -1) or (pattern[j] == pattern[i]):
            i += 1
            j += 1
            if pattern[i] != pattern[j]:
                next[i] = j
            else:
                next[i] = next[j]
        else:
            j = next[j]

    return next

if __name__ == "__main__":

    print findPattern("aaaab")
    print findPattern("abaabc")

You can see your implementation is wrong by examining the simplest case, a pattern P consisted of a repeated single character: "AAAAA". At every point of the failure function, the length of the largest proper prefix of P which is also a proper suffix of P is ever-increasing: [-1, 0, 1, 2, 3] - however your implementation computes [-1, -1, -1, -1, -1].

Translating the pseudocode in the Wikipedia entry into Python gives us:

def kmp_table(pattern):
    if len(pattern) == 0:
        return None

    ff = [0] * len(pattern) # FF for failure function
    ff[0] = -1
    pos = 2
    cnd = 0

    while pos < len(pattern):
        if pattern[pos - 1] == pattern[cnd]:
            cnd += 1
            ff[pos] = cnd
            pos += 1
        elif cnd > 0:
            cnd = ff[cnd]
        else:
            ff[pos] = 0
            pos += 1

    return ff