Ruby on rails action code could be shorter So I have this code that i use for addition of funds to a certain item.

def addFund
    fund=params[:fund]

    if(fund.to_i>0)
      @fund=fund
      if(@item.fund+fund.to_f>@item.price) #the fund added should be less than price-current fudn
        respond_to do |format|

         format.html { redirect_to @item, error: 'Fund amount should not exceed price difference' }
         format.json {render :json =>{:status =>0,
          :error=>"Fund amount cannot be greater than price difference  "}}
        end
        else
          #Finally do the addition
          @item.fund=@item.fund+fund.to_f
          @item.save
         respond_to do |format|

         format.html { redirect_to @item, success: 'Fund  added' }
         format.json {render :json =>{:status =>1,
          :error=>"Done"}}
        end
      end
    else
       respond_to do |format|
         format.html { redirect_to @item, error: 'No money specified.' }
         format.json {render :json =>{:status =>0,
          :error=>"Money not specified"}}
        end
    end

  end

So this is how it works:

First we check if the fund(POST ParameteR) is non zero fund.to_i>0 and respond accordingly giving an error message if it isn't

@item.fund = the current funding the item has

@item.price =  the price of item

fund = the fund to be added(POST)

fund must be less than @item.price-@item.fund  since there is no point in accepting funds greater than the price, the item can already be bought

Then  finally if all this works, then we add the funds to the item and show him a message.

So that's pretty much it. I am thinking there has to be a way of clubbing the responses and replying at one time. I have no idea how respond_to actually works. Ruby has blocks that you won't find in java or C++ or PHP

A lot of these logic could be moved to model. Rails provides lot of helpers for validation in your model.

class Item
  validate :fund_vs_price, :fund_greater_than_zero

  def fund_vs_price
    errors.add(:fund, "Fund amount should not exceed price difference") if fund > price
  end

  def fund_greater_than_zero
    errors.add(:fund, "No money specified.")
  end

  #overriding setter to do a sum every time it is assigned a value
  def fund=(value)
    write_attribute(:fund, fund.to_f + value)
  end
end

With that, the controller becomes really simple.

def addFund
    @item.fund = prams[:fund]
    if @item.save
       respond_to do |format|

         format.html { redirect_to @item, success: 'Fund  added' }
         format.json {render :json =>{:status =>1,
          :error=>"Done"}}
       end
    else
      respond_to do |format|

       format.html { redirect_to @item, error: @item.errors[:fund].join("\n") }
       format.json {render :json =>{:status =>0,
        :error=> @item.errors[:fund].join("\n")}}
      end
    end
  end

If your model becomes too big to maintain, you can always use ActiveSupport concerns to separate it out into different modules. For eg, you could move everything related to validation into a separate module.

There might be some logical errors in the above code since its not tested. But it will give you an idea


Read SIDs and set ACLs The following script reads a set of SIDs inside firewall rules and uses them to set ACL rules in a folder.

The question is: Is this syntax ok or is there better ways to write this script, considering powershell syntax features? (by the way, this code works)

param([Parameter(Mandatory)]$foldername)

$result=""

(Get-NetFirewallRule | Where-Object { ($_.Direction -eq "Outbound") -and ($_.DisplayName -like "*appcontainer*")}) |
         %{ ($_ | Get-NetFirewallApplicationFilter) |
        %{  $acl = Get-Acl $foldername
            $sec= New-Object System.Security.Principal.SecurityIdentifier($_.Package)
            $AccessRule = New-Object System.Security.AccessControl.FileSystemAccessRule($sec,"FullControl",(1 -bor 2),0, "Allow")
            $acl.SetAccessRule($AccessRule)
            $acl | Set-Acl $foldername
          } }

nope! [grin]

this is almost entirely about style, not performance. you will note that most points are about readability ... the following line is quite true ...

better readability gives better understanding gives better maintainability.

[1] never use aliases or short names in a script that will be shared or reused
why? code is READ far more often than written, so make it easy to read.

[2] use consistent indents
your indentation makes it needlessly unclear where each stage begins and ends. see [1] for why that is important.

[3] you need comments describing the purpose of the code
again, someone who reads your code next month [perhaps you] needs some sort of idea as to the intent of your code.

[4] avoid long lines
you have two lines that run well off to the right. since PoSh knows that things like pipe symbols WILL be followed by more stuff, you can wrap after such. that also applies to commas, open parens, and other such "more to come" items.

why bother? once again, it is about reading. there is a reason why most wide-format publications to use multiple columns ... people tend to read more comfortably when the lines require minimal side-to-side eye movement. that works out to about 80 to 100 chars per line.

make use of the vertical space you have available. folks scan vertically somewhat more easily than horizontally.

[5] use consistent variable case
you use lowercase and PascalCase. try to stick with ONE such style ... and PascalCase is the one usually recommended.

[6] read the officially referenced unofficial style & best practices guide
this ...

PoshCode/PowerShellPracticeAndStyle: The Unofficial PowerShell Best Practices and Style Guide
— https://github.com/PoshCode/PowerShellPracticeAndStyle

[7] remove unused code
you define $Result ... but never use it. i would remove that.

[8] define the type for your parameter
your $FolderName parameter type is not defined. i suspect it is [string], tho.

[9] avoid sending things across a pipe if you can
you send $_ to Get-NetFirewallApplicationFilter via a pipe. pipeline ops are slower than direct ops & it looks like you could call that out by using the parameter name directly.

here's how i would rework your code layout ...

Param (
    [Parameter(
        Mandatory)]
        [string]
        $FolderName
    )

Get-NetFirewallRule |
    Where-Object {
        $_.Direction -eq "Outbound" -and
        $_.DisplayName -like "*appcontainer*"
        } |
    ForEach-Object {
        # i wonder if the next two lines could be done as
        #    Get-NetFirewallApplicationFilter -AssociatedNetFirewallRule $_
        $_ |
        Get-NetFirewallApplicationFilter |
        ForEach-Object {
            $Acl = Get-Acl $FolderName
            $Sec= New-Object System.Security.Principal.SecurityIdentifier($_.Package)
            $AccessRule = New-Object System.Security.AccessControl.FileSystemAccessRule(
                $Sec,
                "FullControl",
                (1 -bor 2),
                0,
                "Allow"
                )
            $Acl.SetAccessRule($AccessRule)
            $Acl |
                Set-Acl $FolderName
            } # end 2nd FE-O
        } # end 1st FE-O


Global site settings middleware for Slim Framework 3 I have created this middleware code for slim. I just want to know that is this right way to store site settings globally. so, I can use where ever I want. What do you think about my class structure apart from invoke (which is slim's own feature) what do you think about foreach and setting the data. I'm trying to learn and find good practice. thanks.

this is the middleware

    namespace entity;

    use PDO;
    use Psr\Http\Message\ServerRequestInterface;
    use Psr\Http\Message\ResponseInterface;

    class SiteSettings {

      public function __construct( $app ) {
        $this->pdo =  $app->getContainer()['pdo'];
      }
      public function __invoke( $request, $response, $next ) {
        if( $this->getSettings() !== false ) {
          foreach ( $this->getSettings()  as $row )
          $response = $next($request->withAttribute($row['setting'], $row['value']), $response );
        }
        return $response;
      }
      protected function getSettings() {
        $sql = "SELECT setting, value, description FROM site_settings";
        try {
          $query = $this->pdo->prepare( $sql );
          if ( $query->execute() ) {
            return $query->fetchAll(PDO::FETCH_ASSOC);
          }
          return false;
        } catch ( PDOException $pe ) {
          return false;
        }
      }
    }

added to all routes

$app->add( new entity\SiteSettings( $app ) );

and able to use it like this

    $app->get('/en/admin', function ($request, $response, $args) {
        // Sample log message
        $this->logger->info("Tes '/' route");
        echo $request->getAttribute('site_name');

     });

and this is the table

     CREATE TABLE `site_settings` (
        `setting` VARCHAR(250) NOT NULL DEFAULT '' COLLATE 'utf8_unicode_ci',
        `value` VARCHAR(250) NULL DEFAULT NULL COLLATE 'utf8_unicode_ci',
        `description` TINYTEXT NULL COLLATE 'utf8_unicode_ci',
        PRIMARY KEY (`setting`)
     )
     COLLATE='utf8_unicode_ci'
     ENGINE=InnoDB
     ;

A couple of general comments:

Please follow the PSR coding standards whenever possible
Add type-hints for all arguments, especially constructor arguments. You're clearly expecting $app to be an object. Add the type-hint
Predeclare properties, because: what PHP calls object overloading (dynamic properties) are all public, and are slower. Relying on them makes it easy to remove/reassign core properties and makes your code buggy the moment you have typos, which you will. Inevitably.
Avoid one-line loops without brackets (foreach ( $this->getSettings()  as $row )). I know they're technically not needed there, but add them. It's a good habit that avoids awful bugs (think: goto fail;)
You're using PDO, which is good. You're using prepared statements, which is excellent, but the way you're using them doesn't really make sense
Your use of PDO is made worse by the fact that you're repeatedly calling getSettings. Each call will query the DB for the same data. Don't do that

So Let's start by looking at the constructor, how I'd rewrite it:

//add use Slim\App;
/**
 * @var PDO
 */
protected $pdo = null;

/**
 * You're not using this, but just in case:
 * @var App
 */
 protected $app = null;
public function __construct(App $app)
{//<-- PSR coding style
    $this->app = $app;
    $this->pdo = $app->getContainer()['pdo'];
    if (!$this->pdo instanceof PDO) {
        throw new \RuntimeException(
            sprintf(
                '%s requires a PDO instance, app did not contain "pdo" key',
                __CLASS__
            )
        );
    }
}

Avoid querying too many times

Your __invoke method calls getSettings several times. Each time, you'll be querying for the exact same data as the last time. I'd rewrite that ASAP:

public function __invoke( $request, $response, $next )
{
    $settings = $this->getSettings();//assign!
    if( $settings ) {
        foreach ($settings as $row) {
            //set all attributes first
            $request->withAttribute(
                $row['setting'],
                $row['value']
            );
        }
        $response = $next($request, $response);//only call next once
    }

    return $response;
  }

Because your class is doubling as a callable instance (hence the __invoke), you might want to try adding type-hints to the __invoke method, too.

About your use of prepared statements:

Prepared statements are what you use when you're going to query the DB, and you want to use values that might be used for injection attacks, or values that, because they contain quotes might simply break the query.
You might also choose to use prepared statements for queries that you're running mutliple times, because prepared statements can be reused.

You're doing neither: you're creating a prepared statement resource, execute it and forget about it. The query you want to execute in fact is hard coded, so there's no risk of injection whatsoever. What I'd do is this:

$stmt = $this->pdo->query('SELECT setting, value, description FROM site_settings');
return $stmt->fetchAll(PDO::FETCH_ASSOC);

Nothing more. If an exception is thrown here, that should cause the app to grind to a halt: The query to fetch the site settings failed. There's nothing your app can do that would be even remotely sensible. It's a fatal error, so let the exception propagate until a global handler catches it and renders an error page. Don't hush the exception up.

Right now, you're returning false both if a PDOException was thrown, or if there aren't any settings. That's madness. An EXCEPTION signals something EXCEPTIONAL has happened in a bad way. Don't hush it up, sound the alarms! Your app is on fire!

Final nit-pick

Going by the Slim documentation, and because they say their container component is based on pimple, I'd probably use this to get the PDO instance from the container:

$this->pdo = $app->getContainer()->get('pdo');

You could write $app->getContainer()->pdo, too, but I have a profound aversion for the magic __get and __set methods. So much so, then I often implement them to throw exceptions with the sole purpose to disable overloading (dynamic properties):

public function __get($name)
{
    throw new \RuntimeException(
        sprintf(
            'tried to access property non-%s "%s" as public on %s',
            property_exists($this, $name) ? 'public' : 'existent',
            $name,
            get_class($this)
        )
    );
}

public function __set($name, $value)
{
    throw new \RuntimeException(
        sprintf(
            'tried to set property non-%s "%s" to "%s" on %s',
            property_exists($this, $name) ? 'public' : 'existent',
            $name,
            serialize($value),
            get_class($this)
        )
    );
}


Crank-Nicolson. Advection - Diffusion I am writing an advection-diffusion solver in Python. I am quite experienced in MATLAB and, therefore, the code implementation looks very close to possible implementation in MATLAB. I implemented the same code in MATLAB and execution time there is much faster. AS you may note, I am also (as in MATLAB) trying to use JIT in Python, however, it does not give me any improvements. Thus, I was wondering if you could review the code in terms of optimization for computational speed and give me some advices for future.

import numpy as np
import scipy as sp
from scipy.sparse import spdiags
from scipy import special
import numba as nb

@nb.jit
def adv_diff(D, w, years):
    BC1_top = 1
    F_bottom = 0
    L = 30
    tend = years
    C1_init = 0

    phi = 1
    dx = 0.1
    dt = 0.001
    x = np.linspace(0, L, L / dx + 1)
    N = x.size

    C1_init = C1_init * np.ones((N, 1))
    C1_init[0] = BC1_top

    [AL, AR] = AL_AR_dirichlet(D, w, phi, dt, dx, N)

    C1_old = C1_init

    time = np.linspace(0, tend, tend / dt + 1)

    C1_res = np.zeros((N, time.size))
    C1_res[:, 0] = C1_init[:, 0]

    for i in np.arange(1, len(time)):
        C1_old = update_bc_dirichlet(C1_old, BC1_top)
        B = AR.dot(C1_old)
        C1_new = linalg_solver(AL, B)
        C1_res[:, i] = C1_new[:, 0]
        C1_old = C1_new
        C1_old[1] = BC1_top

    return C1_res

@nb.jit
def linalg_solver(A, b):
    # linalg_solver: x = A \ b
    return np.linalg.solve(A, b)  #

@nb.jit
def update_bc_dirichlet(C, BC_top):
    # update_bc_dirichlet: function description
    C[0] = BC_top
    return C

@nb.jit
def AL_AR_dirichlet(D, w, phi, dt, dx, N):
    # AL_AR_dirichlet: creates AL and AR matrices with Dirichlet BC
    s = phi * D * dt / dx / dx  #
    q = phi * w * dt / dx  #
    e1 = np.ones((N, 1))  #
    AL = spdiags(np.concatenate((e1 * (-s / 2 - q / 4), e1 * (1 + s), e1 * (-s / 2 + q / 4)), axis=1).T, [-1, 0, 1], N, N).toarray()
    AR = spdiags(np.concatenate((e1 * (s / 2 + q / 4), e1 * (1 - s), e1 * (s / 2 - q / 4)), axis=1).T, [-1, 0, 1], N, N).toarray()
    AL[0, 0] = 1
    AL[0, 1] = 0
    AL[N - 1, N - 1] = 1 + s
    AL[N - 1, N - 1 - 1] = -s
    AR[0, 0] = 1
    AR[0, 1] = 0
    AR[N - 1, N - 1] = 1 - s
    AR[N - 1, N - 1 - 1] = s
    return AL, AR

if __name__ == '__main__':
    D = 0.5
    w = 3
    t = 10

    C = adv_diff(D, w, t)

I know nothing about your business logic but you're trying to transpose matlab in Python and it doesn't look like anything: there are some useless parameters, you are building arrays only to take their size. I'll try to provide advices to clean up the code and maybe others will provide runtime improvements.

Remove unused variables like F_bottom or tend which is only an alias to years.
Replaces calls like arr = np.linspace(begin, end, size); var = arr.size with var = size if you don't plan on using the array at all.
Use slice assignment to eliminate visual clutter.
Remove update_bc_dirichlet as you already perform the same kind of operation directly in the loop: stay consistent and see previous point.
Remove linalg_solver as it only indirects a single function call. If you want to avoid lookups in the loop, create an alias as local variable.
Some of the constants should either be extracted out of the function as real constants or defined as parameter with default values.
0 * np.ones(..) is np.zeros(..)
Use docstrings instead of comments to document your functions.

I would also turn adv_diff into a generator, but only because I don't know numpy enough to come up with a more idiomatic code:

import numpy as np
import scipy as sp
from scipy.sparse import spdiags
from scipy import special
import numba as nb

@nb.jit
def adv_diff(D, w, years, L=30, phi=1, dx=0.1, dt=0.001):
    """What does this function do?"""
    N = int(L / dx) + 1
    C1 = np.zeros((N, 1))
    C1[0] = 1

    linalg_solver = np.linalg.solve
    AL, AR = AL_AR_dirichlet(D, w, phi, dt, dx, N)

    yield C1
    for _ in range(int(year/dt)):
        C1 = linalg_solver(AL, AR.dot(C1))
        yield C1
        C1[:2, 0] = 1, 1

@nb.jit
def AL_AR_dirichlet(D, w, phi, dt, dx, N):
    """AL_AR_dirichlet: creates AL and AR matrices with Dirichlet BC"""
    s = phi * D * dt / dx / dx #
    q = phi * w * dt / dx #
    e1 = np.ones((N, 1)) #
    AL = spdiags(np.concatenate((e1 * (-s / 2 - q / 4), e1 * (1 + s), e1 * (-s / 2 + q / 4)), axis=1).T, [-1, 0, 1], N, N).toarray()
    AR = spdiags(np.concatenate((e1 * (s / 2 + q / 4), e1 * (1 - s), e1 * (s / 2 - q / 4)), axis=1).T, [-1, 0, 1], N, N).toarray()

    AL[0, :2] = 1, 0
    AL[-1, -2:] = -s, 1 + s
    AR[0, :2] = 1, 0
    AR[-1, -2:] = s, 1 - s

    return AL, AR

if __name__ == '__main__':
    D = 0.5
    w = 3
    t = 10
    C = np.asarray(list(adv_diff(D, w, t)))


Enum synced with Database Our database has several tables that have a value that can be one of 4 things, so we made these columns instead have an id column with a foreign key to an enum table with an id and a value column to hold those 4 values. When interfacing this through an ORM, I came up with the following enum-like class to manage keeping things in sync:

using DataAccess.ECI.Models;
using System;
using System.Collections.Generic;
using System.Text;
using System.Linq;

namespace DataAccess.ECI
{

    public class StatusEnum
    {
        private string type;
        private static Dictionary<string, Status> lookupCache = null;
        private static List<StatusEnum> members= new List<StatusEnum>();
        private StatusEnum(string type)
        {
            this.type = type;
            members.Add(this);
        }
        //the text needs to be kept consitent with the Status table
        public static readonly StatusEnum ACTIVE = new StatusEnum("ACTIVE");
        public static readonly StatusEnum COMPLETED= new StatusEnum("COMPLETED");
        public static readonly StatusEnum DELETED= new StatusEnum("DELETED");
        public static readonly StatusEnum INPROGRESS= new StatusEnum("INPROGRESS");

        public static void Init(ECIContext ctx)
        {
            lookupCache=new Dictionary<string, Status>();
            foreach(StatusEnum member in members)
            {
                try
                {
                    lookupCache.Add(member.type, ctx.Status.Single(s => s.Name == member.type));
                }catch(Exception ex)
                {
                    throw new InvalidOperationException($"No record found with Name {member.type}",ex);
                }
            }
        }
        public Status asDBType()
        {
            if (lookupCache == null) throw new InvalidOperationException("Init(ECIContext) has not been called for this enum");
            return lookupCache[type];
        }
        public override string ToString()
        {
            return type;
        }
    }
}

I thought it works pretty well, but I'm concerned about a few things.

since it's mostly static methods, it forces lots of duplication when other enum tables are added that need the same code wrapper
if we forget to call init, this error isn't caught until late in the runtime

I would appreciate any comments on the code and suggestions for how to make it better.

EDIT:

for more context, we are using aspnet core, so the context is added in Startup.cs:ConfigureServices(IServiceCollection services) :

...
services.AddDBContext<ECIContext>(options=>
    options.UseSqlServer(/*connection string from config*/)
);
...

and Init is currently called in Startup.cs:Configure(IApplicationBuilder app,...):

...
ECIContext ctx=app.ApplicationServices.GetService<ECIContext>();
StatusEnum.Init(ctx);
...

this enum will be used like

ctx.Document.add(new Document{
    PageCount=4,
    Status=StatusEnum.ACTIVE.asDBType(),
    ...other properties...
});

which works because our ORM, entity framework core, adds a field for the foreign key linked table, and stores the id when appropriate

So your Init threw me off, initially I thought this class was doing something, but it was doing something else (not your fault, totally mine).

Here's how I would handle this:

Get rid of asDBType and just return the Status, we'll see how in a moment.
Don't call the private member type if the database member is Name, call it name.
Use CallerMemberNameAttribute so you don't need a magic string.

You want type-safety at compile time, against something that's inherently unsafe, so we have to cheat quite a bit to make that happen.

We're also going to use some Reflection - doing so means we can eliminate the List<StatusEnum> members, and replace a lot of this class with a couple quick calls.

public class StatusEnum
{
    private static Dictionary<string, Status> _lookup = new Dictionary<string, Status>();
    private static Status _get([CallerMemberName]string name = null) =>
        _lookup.Length > 0 ? _lookup[name] : throw new InvalidOperationException($"The method '{nameof(Init)}' has not been called.");
        // `CallerMemberNameAttribute` gives us the name of whatever
        // method or property called this method, which means since
        // our property calls `_get()`, we get the name of that
        // property implicitly

    public static Status ACTIVE => _get();
    public static Status COMPLETED => _get();
    public static Status DELETED => _get();
    public static Status INPROGRESS => _get();

    public static void Init(ECIContext ctx)
    {
        foreach (var prop in typeof(StatusEnum).GetProperties())
        {
            var name = prop.Name;

            // Assuming `ctx.Status` has a `SingleOrDefault`
            var status = ctx.Status.SingleOrDefault(x => x.Name == name);
            if (status == null) { throw new ArgumentException($"The value '{name}' is not a valid '{nameof(Status)}'."); }

            _lookup.Add(name, status);
        }
    }
}

Realistically, your StatusEnum class is a bunch of helper stuff, so you don't really need instance members, and you don't really want to keep an instance of this class, you care about the Status class. This is similar to how I do Setting objects in Entity Framework, though my setup was more complex.

Your version requires adding a property with a self-referencing name, this version eliminates that.

public static Status ONHOLD => _get();
public static readonly StatusEnum ONHOLD = new StatusEnum("ONHOLD");

Also, in your version, C#6.0 introduced nameof() which would allow you to safely initialize:

public static readonly StatusEnum ACTIVE = new StatusEnum(nameof(Active));

The other advantage to this version is you'll always be sure that you have a valid Status object, no need to asDBType, which means we can make this generic for your other situations:

public abstract class ItemizedEnum<TItem, TClass>
    where TClass : ItemizedEnum<TItem, TClass>
{
    private static Dictionary<string, TItem> _lookup = new Dictionary<string, TItem>();
    protected static TItem Get([CallerMemberName]string name = null) =>
        _lookup.Count > 0 ? _lookup[name] : throw new InvalidOperationException($"The '{nameof(Init)}' method MUST be called first.");

    protected abstract TItem GetFromDatabase(ECIContext ctx, string name);

    public static void Init(ECIContext ctx)
    {
        var classInstance = Activator.CreateInstance<TClass>();
        foreach (var prop in typeof(TClass).GetProperties())
        {
            var name = prop.Name;
            var obj = classInstance.GetFromDatabase(ctx, name);
            if (obj == null || obj.Equals(default(TItem)) == true) { throw new ArgumentException($"The value '{name}' is invalid for '{typeof(TClass).Name}'"); }
            _lookup.Add(name, obj);
        }
    }
}

Finally, in StatusEnum:

public class StatusEnum : ItemizedEnum<Status, StatusEnum>
{
    public static Status ACTIVE => Get();
    public static Status COMPLETED => Get();
    public static Status DELETED => Get();
    public static Status INPROGRESS => Get();

    protected override Status GetFromDatabase(ECIContext ctx, string name) =>
        ctx.Status.FirstOrDefault(x => x.Name == name);
}

This decreases the level of redundancy, the class only needs to implement a GetFromDatabase(ECIContext, string) method, and we now no longer care about any of that in StatusEnum itself, just what values it contains. We can then build other ItemizedEnum classes:

public class StateEnum : ItemizedEnum<State, StateEnum>
{
    protected override State GetFromDatabase(ECIContext ctx, string name) =>
        ctx.State.FirstOrDefault(x => x.Title == name);

    public static State OtherTest => Get();
}

And we get the same type-safety. Now a test of your live examples might be:

public class StatusEnum : ItemizedEnum<Status, StatusEnum>
{
    protected override Status GetFromDatabase(ECIContext ctx, string name) =>
        ctx.Status.FirstOrDefault(x => x.Name == name);

    public static Status Test => Get();
}

public class StateEnum : ItemizedEnum<State, StateEnum>
{
    protected override State GetFromDatabase(ECIContext ctx, string name) =>
        ctx.State.FirstOrDefault(x => x.Title == name);

    public static State OtherTest => Get();
}

StatusEnum.Init(new ECIContext());
StateEnum.Init(new ECIContext());
Console.WriteLine(StatusEnum.Test.Name);
Console.WriteLine(StateEnum.OtherTest.Title);


Version 2 - print new even-length arrays from two arrays This is an update of the code for my previous post (which has now been removed as this code is more efficient and accurate) for the GeeksForGeeks task.

Task outline

Given two arrays, print all new arrays of even length where each subsequent element in the new array comes alternately from A then B and each element is larger than the previous element

Example:

Array 1 = [1, 2, 3]
Array 2 = [2, 4, 6]

Possible new array lengths = [2, 4, 6]

The output for arrays of length 2 is:
[1,2], [1,4], [1,6], [2,4], [2,6], [3,4] [3,6]

The full solution for the above includes the following two additional arrays:
[1 2 3 4], [1 2 3 6]

This solution uses the itertools library

import itertools
A=[10, 20, 30,40]
B=[11,21,31,41]
list_solutions = []

for x in range (1,min(len(A),len(B))+1):
    newA = list(itertools.combinations(A,x))
    newB = list(itertools.combinations(B,x))
    for itemA in newA:
        for itemB in newB:
            to_print = True
            for index in range (min(len(itemA),len(itemB))):
                if itemA[index] >= itemB[index]:
                    to_print = False
                    break
            if to_print == True:
                list_solutions.append([itemA, itemB])

#Print only valid solutions:
for item in list_solutions:
    print_valid = True
    for index in range (len(item[0])):
        if item[0][index] >= item[1][index]:
            print_valid = False
            break
        if index >= 1:
            if item[0][index] <= item[1][index-1]:
                print_valid = False
                break
    if print_valid == True:
        for index in range (len(item[0])):
            print (item[0][index], item[1][index], sep = " ", end = " ")
        print ("")
    if print_valid == False:
        continue

It's unclear to me why you didn't chose to use a tree. From your example you know:

A[0] has the children B[0:].
A[1] has the children B[1:].
A[2] has the children B[1:].
B[0] has the children A[2:].
B[1] has no children.
B[2] has no children.

From this you should be able to see that you'll just have to walk the tree to get the values.

To get all the values you walk the tree with the roots being all the values in A. And you filter odd results.

class Node:
    def __init__(self, value):
        self.value = value
        self.children = []

    def walk(self, path=None):
        path = (path or ()) + (self.value,)
        yield path
        for child in self.children:
            yield from child.walk(path)

def solution(A, B):
    A = [Node(a) for a in A]
    B = [Node(b) for b in B]

    for parents, children in ((A, B), (B, A)):
        for parent in parents:
            parent.children = [
                child
                for child in children
                if child.value > parent.value
            ]

    for start in A:
        for values in start.walk():
            if len(values) % 2 == 0:
                print(values)


Find words with this prefix This function (find_prefix) is supposed to find words starting with a given prefix in a container. The container is sorted. This function is a fairly performance-critical part of the code, so I am open for any optimization suggestions. Here is a brief usage case; test_in is a file with \n separated English words:

#include <iostream>
#include <vector>
#include <algorithm>
#include <string>
#include <iterator>
#include <tuple>
#include <fstream>

template <typename It_beg, typename It_end, typename Prefix>
auto find_prefix(const It_beg beg,
                 const It_end end,
                 Prefix prefix)
{
  auto low = std::lower_bound(beg, end, prefix);
  ++prefix.back();
  auto up = std::lower_bound(beg, end, prefix);
  return std::make_tuple(low, up);
}

int main()
{
  std::string s;
  std::fstream file("test_in");
  if (!file)
  {
    std::cerr << "Failed to open the file\n";
    return -1;
  }

  std::vector<std::string> svec;
  svec.reserve(470000);

  while (file >> s)
  {
    svec.push_back(s);
  }

  std::sort(svec.begin(), svec.end());

  while (std::cin >> s)
  {
    auto [beg, end] = find_prefix(svec.cbegin(), svec.cend(), s);
    std::copy(beg, end, std::ostream_iterator<std::string>(std::cout, "\n"));
  }

  return 0;
}

When the input is "stack" we get this output:

stack
stack's
stack-garth
stackable
stackage
stacked
stackencloud
stacker
stackering
stackers
stacket
stackfreed
stackful
stackgarth
stackhousiaceous
stacking
stackless
stackman
stackmen
stacks
stackstand
stackup
stackups
stackyard

As you are effectively reading the whole file test_in into memory in lots of small independent chunks, consider a different approach:

Read the whole file as one big chunk, or preferably simply map it.
Put std::string_views into that array in the vector, instead of strings.

Most std::strings employ SSO, meaning there are two possible states (external string, active SSO), in contrast to the single one for std::string_view. Thus, no branch-misprediction.

Additionally, std::string is generally at least twice the size of std::string_view, so this about halves the size of the vector's data.
Due to allocator overhead, all the small bits of string data would also together likely need considerably more space than that one big chunk.
You copy the prefix on calling find_prefix(). Better pass a view.
When you know where the range you want starts, you know it won't end earlier.
++prefix.back(); is blatantly wrong.

If the prefix is empty, it's Undefined Behavior.
If it causes signed overflow, it's Undefined Behavior.
If it causes unsigned wrap-around, you simply get a completely wrong end-position, which might even lie before the start-position.

I suggest simply writing your own comparator and using std::upper_bound() too, as it is intended to be used.

Be aware that char is treated as unsigned char for comparison by std::char_traits<char>. You probably want to mirror that, especially as you used that rule when sorting... I won't go to the trouble of staying that general.
std::equal_range() with a custom comparator does what you want:

template <class RandomAccessIterator>
auto find_prefix(RandomAccessIterator first, RandomAccessIterator last, std::string_view prefix) {
    return std::equal_range(first, last, prefix,
        [n = prefix.size()](std::string_view a, std::string_view b) noexcept {
            return a.substr(0, n) < b.substr(0, n);
    });
}

You don't need std::make_tuple in C++17. Just use std::tuple directly:

return std::tuple{low, up};

By the way, return 0; is implicit for main().


C# + Linq - grouping objects by frames of time found in the data A bit of background. In the application I work on there are emails sent to notify users about articles they need to review. Each email+recipient pair is stored in database and may be assigned a different creation date ( called DateCreated ) due to emails being generated over some time.

I needed to calculate how many times the emails were sent and how many recipients were there on each turn. For this reason I grouped emails which come in quick succession into groups. I used a TimeSpan of 30 minutes after each "first" email in a group. In order to do it I needed to store the dates defining groups "somewhere". I used a separate method to wrap my grouping function. The TimeSpan I used is safe, mailing ever only takes up to 2 minutes. Also separate mailings usually happen on separate dates.

My questions here are - Is this easy enough to understand? Could I code it simpler? Any other advice is also welcome.

And yes - this should be fixed in the database, there should be a mailing table there but there isn't and I cannot achieve it easily at the moment.

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

namespace UserCommunication
{
    public class ArticleEmailSummary
    {
        public int ArticleId { get; }
        public int EmailTemplateId { get; }
        public DateTime DateCreated { get; }
        public int RecipientCount { get; }

        private ArticleEmailSummary(IGrouping<DateTime, ArticleEmail> grouping)
        {
            ArticleId = grouping.First().ArticleId;
            EmailTemplateId = grouping.First().EmailTemplateId;
            DateCreated = grouping.Key;
            RecipientCount = grouping.Count();
        }

        public static List<ArticleEmailSummary> List(int ArticleId, int emailTemplateId)
        {
            // get email list from db
            List<ArticleEmail> emails = ArticleEmail.List(ArticleId, emailTemplateId);

            // order it ascending by creation date
            IOrderedEnumerable<ArticleEmail> emailsWithOrder = emails.OrderBy(e => e.DateCreated);

            // group into sublists, each group cachet up to X minutes of emails occuring after the first one
            IEnumerable<IGrouping<DateTime, ArticleEmail>> groups = emailsWithOrder.GroupBy(GetGroupingFunction());

            // convert groupings to summaries and return
            return groups.Select(grouping => new ArticleEmailSummary(grouping)).ToList();
        }

        private static Func<ArticleEmail, DateTime> GetGroupingFunction()
        {
            // I am curious of other dev's opinion on this method :)

            TimeSpan maxMailingTime = TimeSpan.FromMinutes(30);
            DateTime lastDate = DateTime.MinValue;

            return delegate (ArticleEmail email)
            {
                if (email.DateCreated - lastDate > maxMailingTime)
                {
                    // the mailing was not sent within maxMailingTime from the start of first email in series
                    // we will update the lastDate and this will group the emails into a separate group
                    lastDate = email.DateCreated;
                }

                // now we return the value which groups our emails
                return lastDate;
            };
        }
    }
}

Edits:

changed emailTemplateId to be of type int - it was just an enum ( with values such as "articleForApproval", "expiringArticle", etc. ), my bad, this must have been confusing indeed
ArticleEmail.List(...) simply returns List<ArticleEmail> that match the specified type of email and specific article
as requested here, is the ArticleEmail class, a bit simplified for the purpose of this question:

public class ArticleEmail {
    public int ArticleId { get; }
    public int EmailTemplateId { get; }
    public DateTime DateCreated { get; }
    public int RecipientId { get; }

    public static List<ArticleEmail> List( int articleId, int emailTemplateId ) {
        // return instances from db
    }
}

This

private static Func<ArticleEmail, DateTime> GetGroupingFunction()
    {
        // I am curious of other dev's opinion on this method :)

        TimeSpan maxMailingTime = TimeSpan.FromMinutes(30);
        DateTime lastDate = DateTime.MinValue;

        return delegate (ArticleEmail email)
        {
            if (email.DateCreated - lastDate > maxMailingTime)
            {
                // the mailing was not sent within maxMailingTime from the start of first email in series
                // we will update the lastDate and this will group the emails into a separate group
                lastDate = email.DateCreated;
            }

            // now we return the value which groups our emails
            return lastDate;
        };
    }

I would just write as:

 private static readonly TimeSpan maxMailingTime = TimeSpan.FromMinutes(30);
 private static readonly DateTime lastDate = DateTime.MinValue;
 private static readonly Func<ArticleEmail, DateTime> GroupingFunction =
    email => (email.DateCreated - lastDate > maxMailingTime) ? email.DateCreated : lastDate;

To initialize a constant TimeSpan you can just use

new TimeSpan(30, 0, 0, 0);

that is a constant of 30 days. Your way with FromMinutes is intended to compute an floating point, like 3.76 days into a TimeSpan and is much slower and inaccurate, since it's floating point operations, not integer.

While I'm not sure what you want with subtracting DateTime.MinValue, which is actually 0 or 1/1/1.
Do you just want to convert a DateTime to a TimeSpan by computing the TimeSpan from 1/1/1 up to the Date ? Creative, but actually useless.

You can get the same with

  const long maxMailingTicks = 30*60*1000000L; // 30minutes
  if (email.DateCreated.Ticks > maxMailingTicks)

Let's think on.
How can a Date ever be smaller than 30minutes ? Do you really want to deal or Check on Dates in the first 30minutes of the life of Jesus Christ ?

So where do we end up ?

 private static readonly Func<ArticleEmail, DateTime> = email => email.DateCreated;

If we assume, the email was not send to notify of the birth of Jesus, there is nothing left to do.

But if you want to keep your test:

 private static readonly Func<ArticleEmail, DateTime> GroupingFunction = email =>
    email.DateCreated.Ticks > maxMailingTicks ? email.DateCreated : DateTime.MinValue;

To Have this possibly working on Linq to SQL also, you rather use
Expression<Func<ArticleEMail, DateTime>> as the Type of the function. There is nothing else to do than exchanging the return type/type of my field. When dealing with Dates and SQL you have to use some special functions sometimes, there could be a revision necessary, but with just "Func", its definitly impossible ever to become SQL at all.
This would definitly work on SQL:

 private static readonly Expression<Func<ArticleEmail, DateTime>> = email => email.DateCreated;

Actually your code was too long. I just dealed with the last 10 lines.


Filtering input data I created a function that filters data and I want to know if it's fully secure to filter user input data.

function user_input_data($data,$options) {
    $output = null;
    $error_msg = "";
    $check_length = false;
    $options_fetch = array(
    @$length_max = $options["length_max"],
    @$length_min = $options["length_min"],
    $data_type = $options["data_type"],
    );

    if($length_min && $length_max) {
        $check_length = true;
    }

    if($data_type == "only_letters") {

        if (preg_match('/^[\p{Arabic}a-zA-Z]+\h?[\p{Arabic}a-zA-Z]*$/u', $data)) {
             $output = $data;
        }else{
            $output = false;
        }
    }

    if($data_type == "only_letters_numbers") {

        if (preg_match('/^[\p{Arabic}a-zA-Z0-9]+\h?[\p{Arabic}a-zA-Z0-9]*$/u', $data)) {
            $output = $data;
        }else{
            $output = false;
        }
    }

    if($data_type == "string") {
        $output = htmlspecialchars($data,ENT_QUOTES,"UTF-8",true);
    }
    if($data_type == "text") {
        $output = strip_tags($data);
    }
    if($data_type == "integer") {
        if(filter_var($data,FILTER_VALIDATE_INT)) {
            $output = $data;
        }
    }
    if($data_type == "float") {
        if(filter_var($data,FILTER_VALIDATE_FLOAT)) {
            $output = $data;
        }
    }
    if($data_type == "boolean") {
        if(filter_var($data,FILTER_VALIDATE_BOOLEAN)) {
            $output = $data;
        }
    }
    if($data_type == "email") {
        if(filter_var($data,FILTER_VALIDATE_EMAIL)) {
            $output = $data;
        }
    }
    if($data_type == "url") {
        if(filter_var($data,FILTER_VALIDATE_URL)) {
            $output = $data;
        }
    }
    if($data_type == "ip") {
        if(filter_var($data,FILTER_VALIDATE_IP)) {
            $output = $data;
        }
    }
    if($data_type == "array") {
        if(is_array($data)) {
            $data = array_map("htmlspecialchars",$data);
            $output = $data;
        }
    }
    if($data_type == "file_directory") {
        if(file_exists($data)) {
            $output = basename($data);
        }
    }
    if($output) {

    if($check_length) {

        if( ( ( mb_strlen($output) >= $length_min ) && ( mb_strlen($output) <= $length_max ) ) === false) {
            $output = false;
        }
    }
    return $output;

    }
}

// call function
$password = user_input_data(
    $_POST["password"],
    array(
        "length_min" => 8, "length_max" => 24 ,"data_type" => "string"
    )
);

I suggest you should use a configuration array like this:

 $config = [
      ip => FILTER_VALIDATE_IP,
      email=> FILTER_VALIDATE_EMAIL
 ]

and simplify the code like this:

if(in_array($config, $data_type)) {
     if($filter_VAR($data, $config[$data_type]) {
          $output = $data;
     }
}

In your code, length is checked only if both min and max are set. Is it the expected behavior? In some cases, only min or max checks are required.
Generally, you should avoid repeating code, in favor of calling functions. This makes it more readable. Plus, you can create test cases for small functions with predictable results, rather that a big complicated thing.
With security I have no idea what this function is designed to protect against.


Semantically structured HTML5 for a news page I'm developing a website and I cut the images that I will use:

CSS contains the top image

CSS contains the center image (repeat-y)

CSS contains the bottom image

And inside news-center class I have .news-content class. Inside .news-content I have this structure:

CodeReview

Semantic HTML5

But I want to use HTML5 markup, so I made this. Is it right?

CSS contains the top image

News Title

Lorem ipsum doler si amet

          News autor, news tags, etc..

CSS contains the bottom image

As div elements have no meaning, we can ignore them for deciding which markup to use in your case. So your structure would be:

CSS contains the top image

News Title

Lorem ipsum doler si amet

    News autor, news tags, etc..

CSS contains the bottom image

If .news-section represents a section that can contain multiple article elements, this seems to be appropriate. But if it always contains a single news entry, you should omit the section.

It’s recommended to use heading elements of the appropriate rank (instead of h1 everywhere). So your h1 would at least (depending on where you include this snippet) be a h3.

Using the header is fine, but you may also omit it if you don’t need it for something specifically (e.g., for grouping some more header content for the news content).

It’s only appropriate to use CSS for including images if these images are decorative. If they represent relevant content, you should use HTML to include them (e.g., in the img element). If the images are decorative and if you need a separate HTML element to include them, you should use a div instead of p (but if you use the p for some other purpose, it’s of course good to use it for the CSS image in addition, if possible).

So a typical news section that contains several news entries could look like this:

News

Foo

Bar

If this news list represents the main content for that page, you can use the main element, e.g.:


Caesar cipher using ASCII character codes This function should take in a string and a number as parameters and 'rotate' the string to the equivalent alphabet letter that is located at the letter position + 'rotation' parameter.
E.g.: "stackoverflow" --- "fgnpxbiresybj"
def caesar(inputString, rotation = 1):
    zNum = ord('z')
    alphaLen = 26

    result = []
    for c in inputString:
        charNum = ord(c) + rotation
        if charNum > zNum:
            result.append(chr(charNum - alphaLen))
        else:
            result.append(chr(charNum))

    return ''.join(result)

Good things

String building: the new string is built step by step using a list, this is more efficient than concatenating char by char.

To improve

Bug: caesar('a',500) returns Ȼ. You likely want to use the operator % to stay in the range a-z.
PEP 8: variable names should be lowercase, with words separated by underscores. More info.


Optimizing boolean checking method I have written a method that checks if n number of true booleans are present in a boolean[] array. Then I improved it to var args format which is as follows:

public static boolean checkNTrue(int n, boolean... args) {
    assert args.length > 0;
    int count = 0;
    for( boolean b : args ) {
        if(b)   count++;
        if( count > n )
            return false;
    }
    return ( count == n );
}

I implement it  to check if only one of all those booleans is true by using:

boolean result = checkNTrue(1, false, true, false, false);

The above returns true confirming the required case.  But what if there are only two boolean arguments? A simple XOR could return the right result.

And in the case of 3 booleans, return (a ^ b ^ c) ^ (a && b && c); works fine too. So, my question is:

Do I need to write separate methods to check in cases with 2 and 3 variables?

Or will this one method do fine? I invite every kind of criticism through downvotes and comments, but please enlighten me! :-)

Premature optimization is a bad idea.  Before you try to optimize anything, run a profiler to measure where your bottlenecks are.  Unless you are running this function in a tight loop, it's unlikely that this function will be a bottleneck.

The special cases that you would introduce don't come for free:

The more line of code you write, the more bugs you are likely to have, statistically.
Your unit tests also have to address the special cases.
There is a bit of overhead in checking whether your special cases apply.

Taking all that into consideration, it's almost certainly not worthwhile to add special cases.

The assertion assert args.length > 0 might be inappropriate.  Considering this code in a vacuum, there is nothing to guarantee that that assertion is logically true.  Rather, it's a runtime validation check.  Validation failures should throw an IllegalArgumentException.  They shouldn't be assertion failures.  (Assertions can be disabled, defeating your check.)

If this were a non-public method, and you are sure that you've written your code such that all call sites have at least two arguments, then an assertion might be appropriate.

Here's a philosophical question: should checkNTrue(0) be true or false?  I think it should be true.


Counting primes less than n The following code was written to be a submission to this challenge on PPCG.

It uses this algorithm, the Meissal-Lehmer method. I follow the Wikipedia entry pretty naively, except I tried to memoize the phi function.

Please see @Shepmaster's answer for cleaned up, more idiomatic Rust.

I am looking for ways to improve speed. Most improvements I believe will come from optimizing the phi function, which is used recursively. Answers about making my code more idiomatic Rust are also welcome.

I've been compiling with cargo build --release. The code runs in ~3 sec for input 1,000,000 and a lot longer for 10,000,000.

use std::env;

fn main() {
    let args: Vec<_> = env::args().collect();
    let x: usize = (args[1]).trim().parse()
        .expect("expected a number");

    let root: usize = (x as f64).sqrt() as usize;
    let y: usize = (x as f64).powf(0.3333333333333) as usize + 1;

    let sieve_size: usize = x/y+2;//y+1;
    let mut sieve: Vec<bool> = vec![true; sieve_size ];
    let mut primes: Vec<usize> = vec![0; sieve_size ];
    sieve[0] = false; sieve[1] = false;

    let mut a: usize = 0;
    let mut num_primes = 1;

    let mut num_primes_smaller_root: usize = 0;

    // find all primes up to x/y ~ x^2/3 aka sieve_size
    for i in 2..sieve_size {
        if sieve[i] {
            if i<= root {
                if i<=y {
                    a+=1;
                }
                num_primes_smaller_root+=1;
            }

            primes[num_primes] = i;
            num_primes +=1;
            let mut multiples: usize = i;
            while multiples < sieve_size {
                sieve[multiples] = false;
                multiples += i;
            }
        }
    }

    let mut phi_results: Vec<Vec<usize>> = vec![ vec![0; a+1 ];x+1 ];

    let mut p_2: usize = 0;
    for i in a+1..num_primes_smaller_root+1 {
        let mut first_term: usize = 0;
        while primes[first_term]< x/primes[i] {
            //println!("checking {} vs {}...",primes[first_term],x/primes[i]);
            first_term+=1;
        }
        if primes[first_term]==x/primes[i] {
            first_term +=1;
        }
        p_2 += (first_term -1) - i + 1;
    }

    /*println!("y={}; a={}",y,a);
    println!("P2 = {}",p_2);
    println!("phi({},{}) = {}",x, a, phi(x,a,&primes));
    */
    println!("pi({}) = {}", x, phi(x,a,&primes,&mut phi_results)+a-1-p_2);
}

fn phi(x:usize, b:usize, primes:&Vec<usize>, phi_results:&mut Vec<Vec<usize>>)->usize {
    if b==0 {
        return x;
    }
    if phi_results[x][b] != 0 {
        return phi_results[x][b];
    }
    let value: usize = phi(x,b-1,primes,phi_results) - phi(x/primes[b],b-1,primes,phi_results);
    phi_results[x][b] = value;
    return value;
}

I'm kind of new to the site, so please let me know how to improve this post as a whole too, if necessary

Here's my first pass on stylistic feedback.

Unneeded parenthesis around args[1]
Redundant type specifications, everywhere
Remove commented-out code
Add spaces around binary operators (/, +, <=, +=)
Don't have double semicolons at the end of the line
Don't add space inside [], like in vec![]
Split multiple statements on different lines
Add spaces after commas
Add spaces after colons
Basically never use &Vec<T>, use &[T] instead
Don't use an explicit return at the end of function
Typo in the println! (pi vs phi)

I believe that you can change some of the loops to use more iterators, but I haven't figured those out yet.

use std::env;

fn main() {
    let args: Vec<_> = env::args().collect();
    let x: usize = args[1].trim().parse().expect("expected a number");

    let root = (x as f64).sqrt() as usize;
    let y = (x as f64).powf(0.3333333333333) as usize + 1;

    let sieve_size = x / y + 2;
    let mut sieve = vec![true; sieve_size];
    let mut primes = vec![0; sieve_size];
    sieve[0] = false;
    sieve[1] = false;

    let mut a = 0;
    let mut num_primes = 1;

    let mut num_primes_smaller_root = 0;

    // find all primes up to x/y ~ x^2/3 aka sieve_size
    for i in 2..sieve_size {
        if sieve[i] {
            if i <= root {
                if i <= y {
                    a += 1;
                }
                num_primes_smaller_root += 1;
            }

            primes[num_primes] = i;
            num_primes += 1;
            let mut multiples = i;
            while multiples < sieve_size {
                sieve[multiples] = false;
                multiples += i;
            }
        }
    }

    let mut phi_results = vec![vec![0; a + 1]; x + 1];

    let mut p_2 = 0;
    for i in a + 1..num_primes_smaller_root + 1 {
        let mut first_term = 0;
        while primes[first_term] < x / primes[i] {
            first_term += 1;
        }
        if primes[first_term] == x / primes[i] {
            first_term += 1;
        }
        p_2 += (first_term - 1) - i + 1;
    }

    println!("pi({}) = {}", x, phi(x, a, &primes, &mut phi_results) + a - 1 - p_2);
}

fn phi(x: usize, b: usize, primes: &[usize], phi_results: &mut [Vec<usize>]) -> usize {
    if b == 0 {
        return x;
    }

    if phi_results[x][b] != 0 {
        return phi_results[x][b];
    }

    let value = phi(x, b - 1, primes, phi_results) - phi(x / primes[b], b - 1, primes, phi_results);
    phi_results[x][b] = value;
    value
}

When I run this code with 1000000 it takes ~300 milliseconds:

$ cargo build --release && time ./target/release/sieve 1000000
phi(1000000) = 78498

real    0m0.307s
user    0m0.204s
sys     0m0.102s

Note that you need to build the code and time it in a separate pass, to not include the time spent compiling.

Performance-wise, I'm not seeing many obvious changes.

Try to avoid direct slice access when feasible (vector[0]). These accesses are checked at run time to avoid walking off into undefined memory. Iterators guarantee that no undefined memory will be accessed and can sidestep that small performance bit.

Iterator adapters can also allow removing mutability:

let mut p_2 = 0;
for (i, zz) in primes[a + 1..num_primes_smaller_root + 1].iter().enumerate() {
    let i = i + a + 1;
    let first_term = primes.iter().take_while(|&&p| p <= x / zz).count();
    p_2 += (first_term - 1) - i + 1;
}

Then, move that arithmetic on i to the end, and apply more adapters to remove more mutability:

let interesting_primes = primes[a + 1..num_primes_smaller_root + 1].iter();

let p_2 =
    interesting_primes
    .map(|ip| primes.iter().take_while(|&&p| p <= x / ip).count())
    .enumerate()
    .map(|(i, v)| v - 1 - i - a)
    .fold(0, |acc, v| acc + v);


Array-based parameters check I've got a simple JavaScript function that takes multiple types of arguments. Inside the function I'm doing a check on the arguments to determine which way the user called the function:

var build = function() {
    var args = Array.prototype.slice.call(arguments)
    var options = {
        to: {},
        from: {},
        relationship: null
    }

    if (_.isString(args[3]) && !_.isUuid(args[3]) && _.isString(args[4])) {
        options.to.type = args[3]
    } else if (_.isString(args[2]) && !_.isUuid(args[2]) && _.isString(args[3])) {
        options.to.type = args[2]
    }
}

// called like:
build(client, dataObj, 'relationship', 'type', 'name')
// or
build(client, dataObj, 'relationship', '<uuid>')

This looks messy and isn't really scalable. Is there a more succinct way I can write this (perhaps using .map(), .filter(), or .reduce())?

Define the functions that correspond to a given argument configuration separately:

const processStringNumber = (string, number) => console.log(number + ' is a number')
const processStringString= (string, string2) => console.log(string2+ ' is a string')

Put the argument configurations and the corresponding functions in a data structure:

const processingObj = [
  {
    argsTypes: ['string', 'number'],
    process: processStringNumber
  },
  {
    argsTypes: ['string', 'string'],
    process: processStringString
  }
]

Write a helper that does the checks and dispatches the corresponding function:

//Retrieve the types of an array of args (can have different implementations)
const getTypes = (args) => args.map((arg) => typeof arg)

//Calls the appropriate function from 'processingObj'
const process = (args, processingObj) =>
  processingObj
    .find((process) => _.isEqual(process.argsTypes, getTypes(args)))
    .process(...args)

Usage:

process(["aaa", 1], processingObj) // 1 is a number
process(["aaa", "aaa"], processingObj) // aaa is a string

Here is also an ES5 version (via Babel)


Script for removing empty rows in a sheet I often receive Excel worksheets (with empty rows) which I have to process as a pivot table. Because pivot tables don't allow empty rows, I've made this script. Any feedback is welcomed.

' Removes empty rows from an sheet.

Public Sub RemoveEmptyLines()
  Dim lastRow As Long
  Dim currentRow As Long
  Dim emptyLine As Boolean
  Dim answer As Integer
  Dim countDeleted As Integer

  Application.ScreenUpdating = False

  answer = MsgBox("Empty lines will be deleted." & vbLf & _
             "Do you wish to continue?", _
              vbYesNo, "Changing table-structure?")

  If answer = vbYes Then
    ' Minus 1 => The last one we check will be the last
    ' but one row.
    lastRow = Range("A1").SpecialCells(xlCellTypeLastCell).row - 1
    currentRow = 1
    countDeleted = 0
    ' Go until the last but two row! The last row can't
    ' be empty (Result of SpecialCells). So we don't have
    ' to check that.
    While currentRow < lastRow

      emptyLine = True

      Do
        ' When the complete row is empty then delete it.
        ' Table structure might be changed while iterating
        ' through the inner loop. So repeat the check from
        ' the main loop.
        If _
           WorksheetFunction.CountA(Rows(currentRow + 1).EntireRow) = 0 _
        And _
           currentRow < lastRow _
        Then

          Rows(currentRow + 1).EntireRow.Delete
          countDeleted = countDeleted + 1
          lastRow = lastRow - 1

        Else
          ' I case of "not empty" leave the inner loop and
          ' check the next line.
          emptyLine = False

        End If

      Loop Until emptyLine = False

      currentRow = currentRow + 1
    Wend

    MsgBox countDeleted & " rows have been deleted.", _
           vbInformation, "Result"

  End If

  Application.ScreenUpdating = True

End Sub

Your macro does what you intend, but in a very confusing manner.

Whilst running, you're not actually analysing the currentRow but the currentRow + 1. For something like that I'd prefer a name like baseRow because it implies that other calculations will be done on top of it. In any case, it would be much simpler to just go last-row to first and avoid all the line-numbering messiness.

IMO, if you have a macro called RemoveEmptyLines (presumably attached to some descriptively-named Button) then asking the user to confirm that is what they want to do is unnecessary and probably annoying to the User-Experience.

I'm also not a fan of using worksheet functions in VBA, especially when used to determine the current state of the worksheet. Excel is notoriously unreliable at maintaining an accurate up-to-date version of things like usedRange, lastCell etc.

If you want to know the final row across all 10,000-odd columns then you're kinda stuck with it, but I think that's overkill. If somebody sends you a spreadsheet of data, there should be at least one column somewhere which contains data for every row. Might be names, might be ID/Reference numbers, might be dates etc.

I would structure your sub as a procedure that takes a column reference as an argument, or asks the user to input it, then uses that column for the basis of determining your finalRow.

You should also avoid multiple delete actions. It's fine for small worksheets, but try deleting a row in a worksheet with lots of data and it can take a long time, each time. Instead, I would create a range, name it deleteRange and use Union() to add any new "rows to be deleted" to it. Then, you can have just one delete action at the end of your sub and do it all in one go.

Your sub then looks like this:

Option Explicit

Public Sub RemoveEmptyLines()

    Application.ScreenUpdating = False
    Application.EnableEvents = False
    Application.Calculation = xlCalculationManual

    Dim columnReference As String
    columnReference = InputBox("Please input the Letter-Reference of the key data column")

    Dim columnNumber As Long
    On Error Resume Next '/ Check that it is a valid reference
        columnNumber = Columns(columnReference).Column
    On Error GoTo 0

    If Not columnNumber > 0 Then
        MsgBox "Your column reference was not recognised, please try again"
        Exit Sub
    End If

    Dim lastRow As Long, firstRow As Long, currentRow As Long
    Dim lineIsEmpty As Boolean
    Dim deleteRange As Range
    Dim deleteCount As Long

    last Row = Cells(Rows.Count, columnNumber).End(xlUp).Row

    firstRow = 1
    For currentRow = lastRow To firstRow Step -1

        lineIsEmpty = (Cells(currentRow, Columns.Count).End(xlToLeft).Column = 1 And IsEmpty(Cells(currentRow, 1)) And IsEmpty(Cells(currentRow, Columns.Count)))

        If lineIsEmpty Then
            If deleteRange Is Nothing Then Set deleteRange = Cells(currentRow, 1) Else Set deleteRange = Union(deleteRange, Cells(currentRow, 1))
            deleteCount = deleteCount + 1
        End If

    Next currentRow

    If Not deleteRange Is Nothing Then deleteRange.EntireRow.Delete

    MsgBox deleteCount & " rows have been deleted.", vbInformation, "Result"

    Application.ScreenUpdating = True
    Application.EnableEvents = True
    Application.Calculation = xlCalculationAutomatic

End Sub

Succinct, clear, easy to read and understand.


Checking if a cookie is set Upon visiting my website, I check if a cookie is set into the user's browser and proceed to logging him into his account. Currently, I do the cookie check in my init.php file and I am asking where I should put it instead because I don't feel like this is the ideal place to be.

Here is my init.php file:

<?php
session_start();
error_reporting(E_ALL);

require_once "config.php";
require_once "functions/functions.php";
require_once __DIR__ . '/../vendor/autoload.php';

use \App\Core as App;
use \App\Models\Auth\Login as Login;
use \App\Core\Database as Database;

$app = new App\App();

/**
 * Checks if login cookie exists and log in the user
 */
if(isset($_COOKIE['cookie_hash']) && isset($_SESSION['cookie_hash']))
{
    $login = new Login(Database::getInstance());
    $login->cookieLogin($_COOKIE['cookie_hash'], $_SESSION['cookie_hash']);
}

App\FlashMessage::clear();

And yeah this App\FlashMessage::clear(); seems out of place too. Its only purpose is to unset the flash_message session. If you have a suggestion where to move it, I'd appreciate it too. If I get rid of the cookie check, I can then remove these 2 lines:

use \App\Models\Auth\Login as Login;
use \App\Core\Database as Database;

and my code will be much cleaner.

This is how my file structure looks like:

I was thinking about moving it into Public/index.php which only requires the init.php file but I am not sure.

yes its possible you need in index file include the ini file

require_once "../app/function/init.php";

and also need .htaccess something look like that

Options -MultiViews
<IfModule mod_rewrite.c>
RewriteEngine On

RewriteBase MVC/Source Files/public

RewriteCond %{REQUEST_FILENAME} !-d
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-l

RewriteRule ^(.+)$ index.php?url=$1 [L,QSA]

</IfModule>

you can also put php ini file script in index.php file inside public directory

<?php
session_start();
error_reporting(E_ALL);

require_once "../app/functions/onfig.php";
require_once "../app/functions/functions.php";
require_once __DIR__ . '/../vendor/autoload.php';

use \App\Core as App;
use \App\Models\Auth\Login as Login;
use \App\Core\Database as Database;

$app = new App\App();

/**
 * Checks if login cookie exists and log in the user
 */
if(isset($_COOKIE['cookie_hash']) && isset($_SESSION['cookie_hash']))
{
    $login = new Login(Database::getInstance());
    $login->cookieLogin($_COOKIE['cookie_hash'], $_SESSION['cookie_hash']);
}

App\FlashMessage::clear();

you can install session in your App.php file so session start automatically in every file no need to write


Calculate implicit dates I need help to simplify this code about implicit date calculation.

The context: I have sentences with implicit and explicit dates. For instance:

  "From the 5th to the 7th of September 2017, the 12th, 13th and 15th of June, from the 25th to the 31th December, the 1st and the 2nd of January 2019."

A program (not shown here) can parse implicit and explicit dates, so I can get an ordered list (in chronological order) like that:

    history = [
        [5],
        [7, 9, 2017],
        [12],
        [13],
        [15, 6],
        [25],
        [31, 12],
        [1],
        [2, 1, 2019],
    ]

The purpose of the program below is to calculate the dates.

import datetime

class ImplicitDate(object):
    def __init__(self, day: int, month: int = None, year: int = None):
        self.day = day
        self.month = month
        self.year = year

    def __repr__(self):
        cls = self.__class__.__name__
        return f"<{cls}(day={self.day!r}, month={self.month!r}, year={self.year!r})>"

    def __str__(self):
        s = []
        if self.year:
            s.append(f"{self.year:04d}")
        if self.month:
            s.append(f"{self.month:02d}")
        if self.day:
            s.append(f"{self.day:02d}")
        return "-".join(s)

    @property
    def date(self) -> datetime.date or None:
        if self.year is None or self.month is None or self.day is None:
            return None
        return datetime.date(self.year, self.month, self.day)

    def estimate_date(self, future_date: datetime.date) -> datetime.date:
        if self.day is None:
            # weird
            return future_date
        elif self.month is None:
            day = self.day
            month = future_date.month
            year = future_date.year
            date = datetime.date(year, month, day)
            if date < future_date:
                return date
            else:
                month = future_date.month - 1
                if month == 0:
                    month = 12
                    year = future_date.year - 1
                date = datetime.date(year, month, day)
                return date
        elif self.year is None:
            day = self.day
            month = self.month
            year = future_date.year
            date = datetime.date(year, month, day)
            if date < future_date:
                return date
            else:
                year = future_date.year - 1
                date = datetime.date(year, month, day)
                return date
        else:
            day = self.day
            month = self.month
            year = self.year
            date = datetime.date(year, month, day)
            return date

def demo_implicit_dates():
    history = [
        ImplicitDate(5),
        ImplicitDate(7, 9, 2017),
        ImplicitDate(12),
        ImplicitDate(13),
        ImplicitDate(15, 6),
        ImplicitDate(25),
        ImplicitDate(31, 12),
        ImplicitDate(1),
        ImplicitDate(2, 1, 2019),
    ]

    curr = history.pop()
    dates = [curr.date]
    while history:
        top = dates[-1]
        curr = history.pop()
        estimated = curr.estimate_date(top)
        dates.append(estimated)
    dates.reverse()

    for date in dates:
        print(date.isoformat())

    # 2017-09-05
    # 2017-09-07
    # 2018-06-12
    # 2018-06-13
    # 2018-06-15
    # 2018-12-25
    # 2018-12-31
    # 2019-01-01
    # 2019-01-02

Do you think this program is complex or over-designed? How to simplify it?

Do you think this algorithm is well-implemented? Is there something I am missing?

elif self.month is None:
            day = self.day
            month = future_date.month
            year = future_date.year
            date = datetime.date(year, month, day)
            if date < future_date:
                return date
            else:
                month = future_date.month - 1
                if month == 0:
                    month = 12
                    year = future_date.year - 1
                date = datetime.date(year, month, day)
                return date ```

This can be simplified to:

if len(a_date) > 1:
            if (curr_month and a_date[1] > curr_month):
                curr_year -= 1
            curr_month = a_date[1]

Also,

        elif self.year is None:
            day = self.day
            month = self.month
            year = future_date.year
            date = datetime.date(year, month, day)
            if date < future_date:
                return date
            else:
                year = future_date.year - 1
                date = datetime.date(year, month, day)
                return date ```

Can now use a shared statement such as:

if len(a_date) > 1:
            if (curr_month and a_date[1] > curr_month):
                curr_year -= 1
            curr_month = a_date[1]
        if len(a_date) > 2:
            curr_year = a_date[2]

Then,

if self.year is None or self.month is None or self.day is None

Can be simplified using the any() operator which checks a list of items against a condition (in this case if they are not None) then returns true/false depending on if all items satisfied the condition.

day = self.day
            month = future_date.month
            year = future_date.year
            date = datetime.date(year, month, day)

I would turn this into a function as it is repeated again.

The final code after condensing the logic is:

def implicit_dates(history):
    computed_dates = []
    curr_day = None
    curr_month = None
    curr_year = None
    history = reversed(history)
    for a_date in history:
        curr_day = a_date[0]
        if len(a_date) > 1:
            if (curr_month and a_date[1] > curr_month):
                curr_year -= 1
            curr_month = a_date[1]
        if len(a_date) > 2:
            curr_year = a_date[2]
        if (curr_day and curr_month and curr_year):
            computed_dates.append(
                datetime.datetime(curr_year, curr_month, curr_day))
    return reversed(computed_dates)

for historical_date in implicit_dates(history):
    print(historical_date.strftime("%Y-%m-%d"))


Custom error handler for PHP I've written a custom Error handler for use in PHP. What it does is simply capture PHP errors, and logs them using Monolog.

It works, but I can't help that it has too much stuff inside. I could remove the error levels that are not handled by set_error_handler but that's guaranteed to lead to a developer that spends 2 hours wondering "why E_ERRORs are not getting logged"...

Could this function be improved somehow?

set_error_handler(function($errno ,$errstr,$errfile ,$errline, $errcontext) {
    // When we use the error supressing operator (@)
    // error_reporting is temporarily set to 0
    // Do not log anything for that case
    if(ini_get('error_reporting')==0){return;}

    $halt = FALSE;
    // http://php.net/manual/en/errorfunc.constants.php
    switch($errno){
        case E_USER_ERROR:
        case E_RECOVERABLE_ERROR:
            $halt = true;
            $logType = 'error';
            break;

        case E_WARNING:
        case E_USER_WARNING:
        case E_STRICT:
        case E_DEPRECATED:
            $logType = 'warning';
            break;

        case E_NOTICE:
        case E_USER_NOTICE:
            $logType = 'notice';
            break;

            // The following error types cannot be caught by set_error_handler
            // Adding them for reference only
        case E_CORE_WARNING:
        case E_COMPILE_ERROR:
        case E_COMPILE_WARNING:
        case E_CORE_ERROR:
        case E_ERROR:
        case E_PARSE:
            return FALSE;

        // We should never reach this case. But you never know
        case E_ALL:
        default:
            $logType = 'warning';
    }
    Logger::{$logType}($errstr, $errcontext);
    // Halt the execution for errors that should halt
    if($halt)
        exit($errno);
});

Look here it is already tested on live projects error catcher.
Fatal errors can be catch only by 'register_shutdown_function'
You do not track uncatched Exceptions, it can be done by 'set_exception_handler'
If code runout all memory it will not able to log error, but if you does not catch Fatal errors so it is not your case.


Tree ADT implementation in C As a learning exercise, I decided to implement a BST and add functionality to it slowly. I'd appreciate any feedback.

I should also point out that I borrowed the template from the here.

tree.h header file

typedef char treeElement;
typedef struct TreeNodeTag{
  treeElement  element;
  struct TreeNodeTag  *left, *right;
}treeNode;

typedef struct treeCDT{
  treeNode *root;
}treeCDT;

typedef struct treeCDT *treeADT;

treeNode* addTreeHelper(treeNode**, treeElement);
void addTree(treeADT, treeElement);
int destroyTree(treeADT);
int destroyTreeHelper(treeNode*);
void createTree(treeADT*);
void printTree(treeADT);
void printTreeHelper(treeNode*);

Implementation tree.c

treeNode* addTreeHelper(treeNode **root, treeElement element){

  if(!*root){
    treeNode *temp;
    temp = malloc(sizeof(treeNode));
    temp->element = element;
    temp->left = NULL;
    temp->right = NULL;

    /*
    *root =  malloc(sizeof(treeNode));
    (*root)->element = element;
    (*root)->left = NULL;
    (*root)->right = NULL;
    */
    *root = temp;
    return *root;
  }

  if( (*root)->element >= element ){
    addTreeHelper( &(*root)->left, element);
  }
  else{
    addTreeHelper(&(*root)->right, element);
  }

  return *root;
}

void addTree(treeADT tree, treeElement element){
  addTreeHelper(&(tree->root), element);
}

int destroyTree(treeADT tree){
  destroyTreeHelper(tree->root);
  free(tree);
  return 0;
}

int destroyTreeHelper(treeNode* root){
  if(root){
    destroyTreeHelper(root->left);
    destroyTreeHelper(root->right);
    free(root);
  }
  return 0;
}

void createTree(treeADT* tree){
  *tree = malloc(sizeof(treeCDT));
  if(!*tree){
    fprintf(stderr, "Problem allocating tree");
  }
}

void printTree(treeADT tree){
  printTreeHelper(tree->root);
}

void printTreeHelper(treeNode * node){
  if(node){
    printf("%c\n", node->element);
    if(node->left)
      printTreeHelper(node->left);
    if(node->right)
      printTreeHelper(node->right);
  }
}

API Changes

Currently, as a user of your tree, I have to figure out whether or not I need to call addTree, destroyTree, printTree or their corresponding helper functions. These helper functions are implementation details of the tree and they should not be exposed to the user. Ditto with the treeNode struct. Moreover, if you will be dynamically allocating your tree, you should just be doing so inside the createTree function and then returning that pointer back to the user. I also don't see a need for the typedef to a struct treeCDT *. I personally prefer knowing what the exact type I'm dealing with here and treeADT really doesn't tell me anything. Your header file really should be this:

BST.h

typedef char treeElement;
struct treeCDT;

void addTree(struct treeCDT *, treeElement);
void destroyTree(struct treeCDT *);
struct treeCDT *createTree();
void printTree(struct treeCDT *);

Then, in your implementation file, you can implement all your helpers and mark them as static; this way, your user cannot call them from outside code. You can now define your treeNode and treeCT struct here as well:

BST.c

typedef struct TreeNodeTag{
    treeElement  element;
    struct TreeNodeTag  *left, *right;
} treeNode;

typedef struct treeCDT {
    // ...
} treeADT;

static void addTreeHelper(treeNode**, treeElement)
{

}

static void destroyTreeHelper(treeNode*)
{

}

static void printTreeHelper(treeNode*)
{

}

Bug

You have an error that can cause undefined behavior in your create tree function:

void createTree(treeADT* tree){
  *tree = malloc(sizeof(treeCDT)); // error here.
  if(!*tree){
    fprintf(stderr, "Problem allocating tree");
  }
}

Remember that malloc returns a pointer to memory without initializing it; in other words, the values could be anything! Therefore, the value in the root member of your treeADT is not initialized to NULL like it should be. (See below for alternate implementation).

Factor your code out

As it is now, your implementation is pretty limited as there are no searching, removing, or other utility functions. You can imagine that there will be a need to constantly create new tree nodes. I would make that a function (and mark it static).

static treeNode *createTreeNode(treeElement elem)
{
    treeNode *node = calloc(1, sizeof(*node));
    if (node){
        node->element = elem;
    }
    return node;
}

Now your tree creation function can be:

struct treeCDT *createTree()
{
    return calloc(1, sizeof(struct treeCDT));
}

and the insertion function is trivial now:

void addTree(struct treeCDT *tree, treeElement elem)
{
     if (!tree) { return; }

     if (!(tree->root)) {
         tree->root = createTreeNode(elem);
     } else {
         struct treeCDT *copy = tree->root;
         int inserted = 0;

         while (!inserted) {
             if (elem < copy->element) {
                 if (copy->left)
                     copy = copy->left;
                 else {
                     copy->left = createTreeNode(elem);
                     inserted = 1;
                 }
             } else if (elem > copy->element) {
                 if (copy->right)
                     copy = copy->right;
                 else {
                     copy->right = createTreeNode(elem);
                     inserted = 1;
                 }
             } else {
                 inserted = 1;
             }
         }
     }
}


Java conversion of base 16 to base 64 I wrote a solution in Java to solve the problem here. I use BigInteger to convert first from base 16 to base 10, then base 10 to base 64.

This seems quite inefficient to me, and I would have no idea how to do this if I couldn't use BigInteger.  I'm sure there's a nice and clever way to do this using a lot more bit manipulation.

import java.math.BigInteger;

public class Base64{

    //according to RFC 4648 and RFC 2045
    public final static char[] base64alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/".toCharArray();
    public final static char base64pad = '='; //unused

    public static void main(String[] args){

        String test = "49276d206b696c6c696e6720796f757220627261696e206c696b65206120706f69736f6e6f7573206d757368726f6f6d";
        System.out.println( base16to64( test ) );
    }

    /**Return the base 64 value of an input base 16 string.
     */
    public static String base16to64(String hex){
        BigInteger value = base16to10(hex); //base 10 value

        // --- convert base 10 to 64 ---
        int digits = 1; // # digits in base 64 we'll need
        while( value.compareTo(new BigInteger("64").pow(digits)) != -1 ){ //while the decimal value >= 64^i
            digits++;
        }

        String ret = "";
        for(int i = digits - 1; i >= 0; i--){ //start at most significant digit

            //this digit must be an A (represents 0)
            if( value.subtract(new BigInteger("64").pow(i)).compareTo(BigInteger.ZERO) == -1){
                ret += "A";
            }
            else{
                for(int d = 63; d >= 1; d--){
                    //digit found: [ value - (64^i)*d >= 0 ]  with d the greatest possible
                    if( value.subtract(new BigInteger("64").pow(i).multiply(
                            new BigInteger(Integer.toString(d)))).compareTo(BigInteger.ZERO) != -1){
                        value = value.subtract(new BigInteger("64").pow(i).multiply( new BigInteger(Integer.toString(d))));
                        ret += base64alphabet[d];
                        break;
                    }
                }
            }//end else
        }

        return ret;
    }//end base16to64()

    /**Return the BigInteger decimal value of an input hexadecimal string.
     */
    public static BigInteger base16to10(String hex){
        hex = hex.toLowerCase();

        //hex is 0-9 then a-f
        BigInteger value = BigInteger.ZERO; //base 10 value
        for(int i = 0; i < hex.length(); i++){ //moving from most significant digit towards 16^0

            int digit = (int) hex.charAt(i);
            //if the hex digit is 0-9
            if( '0' <= hex.charAt(i) && hex.charAt(i) <= '9')
                digit = digit - '0';
            //else the hex digit is a-f
            else
                digit = digit - 'a' + 10;

            BigInteger temp = new BigInteger("" + digit); // digit as BigInt
            temp = temp.multiply( new BigInteger("16").pow( hex.length() - 1 - i) ); // digit * 16^index
            value = value.add(temp);
        }

        return value;
    }//end base16to10()
}

You are working too hard. Java has a Base64 class that does most of the work for you, all you need to do is combine it and the constructor for BigInteger, and you are done.

public static String base16to64(String hex){
    return Base64.getEncoder().encodeToString(new BigInteger(hex, 16).toByteArray());
}

For the linked example input:

  49276d206b696c6c696e6720796f757220627261696e206c696b65206120706f69736f6e6f7573206d757368726f6f6d

We get the correct output:

  SSdtIGtpbGxpbmcgeW91ciBicmFpbiBsaWtlIGEgcG9pc29ub3VzIG11c2hyb29t

If you wanted to implement the base 64 conversion yourself, note that you are still doing too much work. Your base16to10 function could be replaced with

BigInteger value = new BigInteger(hex, 16);

One function completely cut out.

However, if you don't want to use BigInteger at all (this is generally a bad idea, but for programming practice, you might want to), notice that log(64)/log(16) == 1.5. Thus, for every 1.5 hex digits, we can obtain 1 base 64 digits.

So take the last 2 hex digits of the hex string: 6d. In binary, this is 01101101. We only need to consider 101101, which is 45. So we take the 45th element of the base 64 alphabet: t. That's the last digit of the base 64 string. Now just shift the hex string over by 6 bits, and repeat (note, shifting by 6 bits won't be clean, so it's probably easier to instead process 3 hex digits to 2 base 64 digits, and take off the last two elements of the hex string).

But noticing the "Cryptopals Rule":

  Always operate on raw bytes, never on encoded strings. Only use hex and base64 for pretty-printing.

You should probably parse the hex string into a byte[] or ArrayList<Byte> before working on it. Honestly, I think this is way easier to do using BigInteger.


C++ implementation of queue using array I have written this code after studying from Introduction to Algorithm. I think this code is very basic and simple. I want to modify it.

#include <iostream>
int tail;
int head;

void initialise_head_tail(){
tail=0;
head=0;
}

void enqueue(int a[],int x,int s){
a[tail]=x;
if( tail+1==s)
    tail=0;
else
    tail++;
}

int dequeue(int a[],int s){
int x=a[head];
if(head+1==s)
    head=0;
else
    head++;
return x;
}
int main()
{
    initialise_head_tail();
    int a[5];
    enqueue(a,15,5);
    enqueue(a,6,5);
    enqueue(a,9,5);
    enqueue(a,8,5);
    enqueue(a,4,5);
    for(int i=0;i<5;i++){
        std::cout<<a[i]<<" ";
    }
    std::cout<<"\n"<<dequeue(a,5);
    return 0;
}

I want to learn if I can apply some better logic. I was thinking to use vector instead of array but there are already methods push_back() and pop_back(). Can we use vector?

Distributing the internal state of your queue over some globals and two arguments is an interesting decision. At least that way you can get the worst of all worlds.
enqueue assumes the queue is not full, dequeue assumes it is not empty. That's also a questionable decision.
You know that streaming a char is more efficient than streaming a length-1-string?
return 0; is implicit for main() in C++ and C99+.
I wonder why you wrap a conditional statement into its own block exactly one time. Some advocate using a block everywhere, others only where needed, but both camps are united in wanting consistency.
The inside of your first three functions somehow lost a level of indentation... and there should be a newline before main().


Group items by types: replace nested for loops with reduce I have a working code that groups items by their type values. The types can be one or multiple, however they are always returned as an array.

To illustrate: if an item has types with values type1 and type2, it should be included in both type1 and type2 properties of the resulting grouped object.

Although the code works, I was wondering if it would be possible to rewrite it without using nested for loops, to make it more aligned with the principles of functional programming, ideally using reduce. Since the data sets are not too large, performance is a secondary consideration, but should be taken into account if possible.

The code is as follows. getTypes is added primarily to show how the types are extracted and ideally shouldn't be changed.

const items = [{
    name: 'First item',
    types: [{
        value: 'type1',
      },
      {
        value: 'type2',
      },
    ],
  },
  {
    name: 'Second item',
    types: [{
        value: 'type1',
      },
      {
        value: 'type3',
      },
    ],
  },
  {
    name: 'Third item',
    types: [{
        value: 'type1',
      },
      {
        value: 'type2',
      },
    ],
  },
];

const getTypes = item => item.types.map(type => type.value);

const groupedItems = {};

for (const item of items) {
  for (const type of getTypes(item)) {
    groupedItems[type] = [...(groupedItems[type] || []), item];
  }
}

console.log(JSON.stringify(groupedItems, null, 4));

Code output:

{
    "type1": [
        {
            "name": "First item",
            "types": [
                {
                    "value": "type1"
                },
                {
                    "value": "type2"
                }
            ]
        },
        {
            "name": "Second item",
            "types": [
                {
                    "value": "type1"
                },
                {
                    "value": "type3"
                }
            ]
        },
        {
            "name": "Third item",
            "types": [
                {
                    "value": "type1"
                },
                {
                    "value": "type2"
                }
            ]
        }
    ],
    "type2": [
        {
            "name": "First item",
            "types": [
                {
                    "value": "type1"
                },
                {
                    "value": "type2"
                }
            ]
        },
        {
            "name": "Third item",
            "types": [
                {
                    "value": "type1"
                },
                {
                    "value": "type2"
                }
            ]
        }
    ],
    "type3": [
        {
            "name": "Second item",
            "types": [
                {
                    "value": "type1"
                },
                {
                    "value": "type3"
                }
            ]
        }
    ]
}

Poor memory usage
Javascript has managed memory. That means it does all the hard work of allocating and releasing memory, which is great (managing memory manually is a real pain).
But this useful feature comes with a down sides...

Managed memory environments are inherently slow,
Managed memory environments encourage very poor memory usage patterns

Looking at your memory and CPU usage.
In your code you create a total of 9 arrays, keeping only 3 of them. (using the data example you provided)
Each time you add an item to a group you create a new array, that must be iterated over when you copy the existing group. groupedItems[type] = [...(groupedItems[type] || []), item]
There are only 6 type values yet you iterate over them 12 times. Once each with item.types.map(type => type.value); and then again with for (const type of getTypes(item))
For the 6 items you add to the 3 groups your code needed 16 iterations.
Less overhead
The example avoids copying the item.types array for each item and assigns a new array to new groups. If a group has been defined the item added is pushed rather than copy the whole array.
The resulting code only needs 6 iterations executing in half the time and using several times less memory
function groupTypes(items) {
    const groups = {};
    for (const item of items) {
        for (const {value: type} of item.types) {
            groups[type] ? groups[type].push(item) : groups[type] = [item];
        }
    }
    return groups;
}

Or slightly quicker by avoiding the copy of the type.value string to to the variable type
function groupTypes(items) {
    const grps = {};
    for (const item of items) {
        for (const type of item.types) {
            grps[type.value] ? grps[type.value].push(item) : grps[type.value] = [item];
        }
    }
    return grps ;
}


Recursive Django filter to output a list from user hierarchy I need to create an HTML unordered list (to display with jstree) to display the hierarchy of users, along with a list underneath the user's "subusers" of their comments. Each user could have many users underneath him, and each of those users could have the same, so I used a recursive filter to accomplish this. However, this has lead to code that feels very repetitive, besides small differences, and it seems to me that surely something could be reduced. I have already identified some repeated code (extracted to _output_comments below), but I would hope I could do more somehow.

Note before the code: User has a foreign key to itself, with subusers as the related_name, and Comment has a foreign key to User.

from django import template

register = template.Library()

@register.filter
def unordered_user_list(user):
    """
    Takes a user and returns an HTML unordered list (without surrounding
 tags) of
    users and comments under it.
    """
    html = ""
    if len(user.subusers.all()) > 0:
        html += "
" + user.name
        if len(user.comment_set.all()) > 0:
            html += "
"
            for subuser in user.subusers.all():
                html += unordered_user_list(subuser)
            html += "
Comments
"
            html += _output_comments(user.comment_set.all())
        else:
            for subuser in user.subusers.all():
                html += "
" + unordered_user_list(subuser) + "
"
        html += "
"
    elif len(user.comment_set.all()) > 0:
        html += "
" + user.name + "

Comments
"
        html += _output_comments(user.comment_set.all())
    else:
        html += "
" + user.name + "
"
    return html

def _output_comments(comment_list):
    output = ""
    for comment in comment_list:
        output += "
" + comment.name + "
"
    output += "

"
    return output

Finally, I would appreciate any other pointers, since this is my first filter. It seems straightforward enough, but for all I know this could be a completely inappropriate use of a filter.

First of all, you can move the first
 out of the first if condition. That allows you to totally get rid of the last else:

html = "
" + user.name
if len(user.subusers.all()) > 0:
    # ...
elif len(user.comment_set.all()) > 0:
    html += "

Comments
"
    # ...
    # don't add the last
 here
html += "
" # add it there instead
return html

Then, _output_comments should handle the whole comment stuff, including the heading:

def _output_comments(comment_list):
    output = "
Comments
"
    for comment in comment_list:
        output += "
" + comment.name + "
"
    output += "

"
    return output

I think that the way you add

 and

 tags does not reflect the structure of the page. You should improve this:

def _output_comments(comment_list):
    output = ("
" +
              "Comments" +
              "
")

    for comment in comment_list:
        output += "
" + comment.name + "
"

    output += ("
" +
               "
" +
               "
")
    return output

Overall, you should try to write Python code whose structures more or less reflects the structure of the HTML code you are writing. That will avoid tricky mistakes and tag mismatches. When I read your code as is, I have trouble knowing whether there are tags mismatches or not.


AJAX error function using a constant I have the following ajax call and if it errors, I have a default error message stored in a constant which I'll use if the response doesn't contain a custom message. When I look at this, I keep thinking it could be done better. Can anyone suggest how I can do it better?

function requestService(){
            $.ajax({
                dataType: 'json',
                type: 'POST',
                url: '/myurl',
                contentType: CONTENT_TYPE,
                timeout: AJAX_TIMEOUT,
                success: function(data, textStatus, jqXHR){
                    populateData(JSON.parse(jqXHR.responseText))
                },
                error: function(err){
                    var errorMessage = AJAX_ERROR;
                    try {
                        errorMessage = JSON.parse(err.responseText).messages.messages[0].description;
                    }
                    catch(error) {
                        //There was an problem finding an error message from the server. The default message will be used.
                    }
                    displayError(errorMessage)
                },
                complete: function(){
                    console.log("complete");
                }
            });
        };

I'd say it's pretty much by-the-book already, but you could use the catch block to set the default:

error: function(err){
  var errorMessage;
  try {
    errorMessage = JSON.parse(err.responseText).messages.messages[0].description;
  } catch(exception) {
    errorMessage = AJAX_ERROR;
  }
  displayError(errorMessage);
}

Or, you can use the full try...catch...finally formulation, if you want:

error: function(err){
  var errorMessage;
  try {
    errorMessage = JSON.parse(err.responseText).messages.messages[0].description;
  } catch(exception) {
    errorMessage = AJAX_ERROR;
  } finally {
    displayError(errorMessage);
  }
}

Or you can try getting the custom error message, and only defaulting at the last moment:

error: function(err){
  var errorMessage;
  try {
    errorMessage = JSON.parse(err.responseText).messages.messages[0].description;
  } catch(exception) {
    // ignored
  }
  displayError(errorMessage || AJAX_ERROR);
}

The possible advantage of this, is that if the server for any reason sends a blank (false'y) error message the above will still default to the generic error message, even though no exceptions were thrown.

In any event the overall approach seems OK to me. There's not really a cleaner way of getting the server's error message, since you have to parse the JSON and dig deep into the structure. Either thing may throw exceptions, so try...catch is the simplest way of handling that. At minimum you need the try..catch for JSON.parse anyway, so you might as well use it for both things.


Query for combining two SELECTs to get active users and 1 inactive user What this query does:

Gets multiple users with the first select in the union; those are the active users.
But when a user is not in that list, but there is a @userid, it should get that user as well and set active to 0 there; that is what the second select does.
If a user is in the first select, and in the second select, the second one should not be in the resultset; that is what the group by with the max is for.

Is there a more efficient way to write this query?

SELECT pk_userid,
       username,
       CAST(MAX(active) AS BIT) AS active
FROM   (SELECT     pk_userid,
                   username,
                   1 AS active
        FROM       dbo.tbluser u
        INNER JOIN tblCustomer c ON u.pk_userid = c.fk_userid
        WHERE      ( c.fk_projectid = @projectid
                 AND c.fk_orderid = @orderid )
        UNION
        SELECT pk_userid,
               username AS username,
               0        AS active
        FROM   dbo.tbluser u
        WHERE  pk_userid = @userid) a
GROUP  BY pk_userid,
          username
ORDER  BY username

Using a CTE will make this query a lot cleaner and probably speed it up a bit as well.

WITH UnionTable AS
(
    SELECT     pk_userid,
               username,
               1 AS active
    FROM       dbo.tbluser AS u
        INNER JOIN tblCustomer AS customer ON u.pk_userid = customer.fk_userid
    WHERE      ( customer.fk_projectid = @projectid
                 AND customer.fk_orderid = @orderid )
    UNION
    SELECT u.pk_userid,
           u.username AS username,
           0        AS active
    FROM   dbo.tbluser AS u
    WHERE  u.pk_userid = @userid
)
SELECT
        pk_userid
        , username
        , CAST(MAX(active) AS BIT) AS active
FROM UnionTable
GROUP BY pk_userid, username
ORDER BY username

The only thing that concerns me is that you label 2 tables in the first select (nested query) but you don't specify which table you want the pk_userid or username to come from, are they only available in the user table? Even if they only exist in the one table, you should get used to using the aliases you create so that it is clear where the columns are being called from.

You should also make sure that your aliases are meaningful as well.  I changed c to customer but I left u as u because user is a reserved word in SQL