Forum project using OOP PHP I've been developing a forum as my first OOP project, I've successfully built a login/registration and other functionality typical in forums.

I'm looking for some guidance in terms of where to take the build and other common functions that will need integrating.

users.php

<?php
class USER
{
  private $db;

  function __construct($DB_con)
  {
    $this->db = $DB_con;
  }

  public function register($fname,$lname,$uname,$umail,$upass)
  {
   try
   {
     $new_password = password_hash($upass, PASSWORD_DEFAULT);

     $stmt = $this->db->prepare("INSERT INTO users(user_name,user_email,user_pass)
       VALUES(:uname, :umail, :upass)");

     $stmt->bindparam(":uname", $uname);
     $stmt->bindparam(":umail", $umail);
     $stmt->bindparam(":upass", $new_password);
     $stmt->execute();

     return $stmt;
   }
   catch(PDOException $e)
   {
     echo $e->getMessage();
   }
 }

 public function login($uname,$umail,$upass)
 {
   try
   {
    $stmt = $this->db->prepare("SELECT * FROM users WHERE user_name=:uname OR user_email=:umail LIMIT 1");
    $stmt->execute(array(':uname'=>$uname, ':umail'=>$umail));
    $userRow=$stmt->fetch(PDO::FETCH_ASSOC);
    if($stmt->rowCount() > 0)
    {
     if(password_verify($upass, $userRow['user_pass']))
     {
      $_SESSION['user_session'] = $userRow['user_id'];
      return true;
    }
    else
    {
      return false;
    }
  }
}
catch(PDOException $e)
{
 echo $e->getMessage();
}
}

public function is_loggedin()
{
  if(isset($_SESSION['user_session']))
  {
   return true;
 }
}

public function redirect($url)
{
 header("Location: $url");
}

public function logout()
{
  session_destroy();
  unset($_SESSION['user_session']);
  return true;
}

public function back() {
  $previous = $_SERVER['HTTP_REFERER'];
  if (isset($previous)) {
    echo $previous;
  } else {
    return FALSE;
  }

}

}
?>

forum.php

<?php

include_once 'database.php';

class FORUM {

    function __construct($DB_con)
    {
        $this->db = $DB_con;
    }

    public function getColumn() {
        try {
            $query = $this->db->prepare("DESCRIBE posts");
            $query->execute();
        //retrieve the columns inside the table posts
            $forumrows = $query->fetchAll(PDO::FETCH_COLUMN);
        //Output each column  Value
            foreach($forumrows as $forumrow) {
                echo $forumrow . "</br>";
            }
        } catch(PDOException $e) {
            echo $e->getMessage();
        }
    }

public function getColumnUrl() {

}

} //End forum class

$forum = new FORUM($DB_con);
$forum->getColumn();

?>

database.php

<?php

session_start();

$DB_host = "localhost";
$DB_user = "root";
$DB_pass = "";
$DB_name = "dblogin";

try
{
    $DB_con = new PDO("mysql:host={$DB_host};dbname={$DB_name}",$DB_user,$DB_pass);
    $DB_con->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
}
catch(PDOException $e)
{
    echo $e->getMessage();
}

include_once 'users.php';
$user = new USER($DB_con);

?>

The rest of the pages just optimise the the functions and a general bootstrap layout for the posts and login pages.

back: I'm not sure what this function is used for (and the name doesn't seem that great), but it is open to XSS. An attacker could create a page at http://evil.com/?<img src=onerror=alert(1)>, which would redirect the user to your back page, which would then echo the XSS payload.
you should always die after a header redirect (clients do not have to follow it, so any code after the redirect may be executed, which may have security implications).
your indentation is inconsistent, and 1 or 2 spaces aren't really enough. I would use 4 at a minimum to ensure readability.
your spacing is also off, and your positioning of curly brackets isn't consistent. You can easily solve all these formatting issues with any IDE.
try to always write out your variable names. Right now, fname may be clear to you (in context), but firstName is much easier to read.
a nested if-else in an if is often not so nice to read (especially with your formatting). It's difficult to see what happens when rowCount is not greater than 0. You could just combine the two ifs.
getColumn doesn't actually get anything, it echoes something. You could rename it to showColumn, but really, it should return an array of columns, which is then echoed in a view class.
the same goes for all the exceptions. Just catching them and echoing the message isn't a great way to handle it. If you can't recover in the function, just throw them upwards, and let the calling code decide what to do.


Fast way to calculate source response using Green's function I am a newbie in the scientific computing in python. The function I am trying to calculate is the response of the wave equation for a given source term (For reference, See: Equation 11.67 in https://webhome.phy.duke.edu/~rgb/Class/phy319/phy319/node75.html).

$$\phi(\pmb{x},t) = \int_V G_+(\pmb{x}, t; \pmb{x'}, t)\rho(\pmb{x'}, t')d^3x'dt'$$

To give context, the term phi in the equation 11.67 is the displacement and the term rho can be thought as a source of disturbance.

Now, in my case, the problem is constructed in spatial dimension of 2 (x-y). Thus, I have to iterate the equation for grid points in x, y and t. This makes the overall calculation extremely time-consuming. Below is my lines of code:

import numpy as np

# Assigning grid points

Nx = 100
Ny = 100
Nt = 100

x = np.linspace(-Nx, Nx, 100)
y = np.linspace(-Ny, Ny, 100)
t = np.linspace(0, Nt-1, 100)

# Equation to solve
# phi(x,y,t) = ∫∫∫ G(x,y,t; x',y',t') . Source(x',y',t') dx' dy' dt'
# G(x,y,t; x',y',t') = Green's Function

# phi = displacement by the wave
phi = np.zeros((Nt,Ny,Nx))

# Define Function to realize Green's Function for Wave Equation
def gw(xx, yy, tt, mm, nn, ss):

    numer = (tt-ss) - np.sqrt((xx-mm)**2+(yy-nn)**2)
    denom = ((tt-ss)**2-(xx-mm)**2-(yy-nn)**2)

    if denom < 0:
        denom = 0
    else:
        denom = np.sqrt(denom)

    kk = np.heaviside(numer,1)/(2*np.pi*denom+1)
    return (kk)

# Define Function to realize Gaussian Disturbance
def g_source(xx, yy, tt):

    spatial_sigma  = 5   # spatial width of the Gaussian
    temporal_sigma = 30  # temporal width of the Gaussian
    onset_time     = 20  # time when Gaussian Disturbance reaches its peak

    kk = np.exp((-(np.sqrt((xx)**2+(yy)**2)/spatial_sigma)**2)))*np.exp(-((tt-onset_time)/temporal_sigma)**2)
    return (kk)

for k in range (Nt):
    for j in range (Ny):
        for i in range (Nx):

            # this is the Green's function evaluated in each grid points
            green = np.array([gw(x[i],y[j],t[k],i_grid,j_grid,k_grid) for k_grid in t for j_grid in y for i_grid in x])
            green = green.reshape(Nt, Ny, Nx)

            # this is the source term (rho), here it is a Gaussian Disturbance
            gauss = np.array([g_source(i_grid,j_grid,k_grid) for k_grid in t for j_grid in y for i_grid in x])
            gauss = gauss.reshape(Nt, Ny, Nx)

            # performing integration
            phi[k,j,i] = sum(sum(sum(np.multiply(green,gauss))))

Please let me know if there is a faster way to do this integration. Thanks in advance. I deeply appreciate your patience and support.

Performance wise

In general:

You use numpy, but you write it almost like in fortran. Python with numpy is good for scientific programing and computing as long as you don't do many loops

read this first A beginners guide to using Python for performance computing
if you really need to do tight loops than use
cython.
But most of the time you can avoid that by expressing the loop as some functional operation on the whole array

most numpy functions can operator on whole array in one pass, use it.

always prefer y[:]=np.sqrt(x[:]) and avoid

          `for i in xrange(N):
                y[i]=np.sqrt(x[i])`

operation like dot,convolve,einsum,ufunc.reduce are very potent to express most sciencetific algorithms in more abstract way
use advanced array slicing

you can express most inner if by boolen mask arrays or integer index array

e.g. f[ f>15.0 ] = 15.0 clamp all elements in array f which are >15.0 to 15.0; you can also store the boolean mask mask = (f>15.0) and than use it like f[mask]+=g[mask]. This way you can express branching as fully functional programs/expressions with arrays.
Do not construct new np.array (np.zeros, np.ones etc.) too often (i.e. inside tight loops). Optimal performance is obtained if you prepare all arrays you need at the beggining. Avoid frequent conversion between list and array

To address your code example in particular:

Green's functions are basically convolutions. I'm pretty sure you can express it using e.g. scipy.ndimage.filters.convolve
if your convolution kernel is large (i.e. pixels interact more than with few neighbors) than it is often much faster to do it in Fourier space (convolution transforms as multiplication) and using np.fftn with O(nlog(N)) cost.
cannot def gw(xx, yy, tt, mm, nn, ss): operate on the whole array,
rather than individual numbers?
if denom < 0: can be expressed using boolean mask array like mask = denom >0; denom[mask] = np.sqrt(denom[maks])
Never do this if you can:

  for k in range (Nt):
     for j in range (Ny):
        for i in range (Nx):

the operations seems to be possible to rewrite that operate on the whole x,y space at once

This is how you do convolution-like operations in numpy (from here)

           # The actual iteration
           u[1:-1, 1:-1] = ((u[0:-2, 1:-1] + u[2:, 1:-1])*dy2 +
                            (u[1:-1,0:-2] + u[1:-1, 2:])*dx2)*dnr_inv

this is so horrible:

green = np.array([gw(x[i],y[j],t[k],i_grid,j_grid,k_grid) for k_grid in t for j_grid in y for i_grid in x])

list comprehesion is relatively fast, but still much slower than numpy array operations (which are implemented in C).
do not create temporary list and convert it to temporary array, you loose lot time doing that.
why you cannot just prealocate Green[Nx,Ny,:] and  Gauss[Nx,Ny,:] and use it as a whole?


Selection Sort in C++ I wanted feedback/review on my implementation of a selection sort in c++ (for ints only, no templates). This is my first attempt in c++ making anything beyond simple projects/hello world!

Q: Should my for loops use uniform initialization like for(int i { 0 }; ... ?

Q: Should everything else use uniform initialization like I have below?

Q: I read selection is n^2 however I don't loop every field, I loop through only the unsorted portion, is this still n^2 or better?

#include <iostream>

void selectionSort(int arr[], int start, int size);
void printArr(int arr[], int size);

int main()
{
  const int size { 11 };
  int arr[size] { 1,9,2,3,4,1,6,6,5,3,8 };
  selectionSort(arr, 0, size);
  return 0;
}

void selectionSort(int arr[], int start, int size)
{
  printArr(arr, size);

  for (int i = start; i < size; i++)
  {
    int min_pos { i };
    for (int x = i; x < size; x++)
    {
      if (arr[x] < arr[min_pos])
      {
        min_pos = x;
      }
    }
    int temp { arr[i] };
    arr[i] = arr[min_pos];
    arr[min_pos] = temp;
    printArr(arr, size);
  }

  printArr(arr, size);
}

void printArr(int arr[], int size)
{
  for (int i = 0; i < size; i++)
  {
    std::cout << arr[i] << " ";
  }
  std::cout << std::endl;
}

output:

1 9 2 3 4 1 6 6 5 3 8
1 9 2 3 4 1 6 6 5 3 8
1 1 2 3 4 9 6 6 5 3 8
1 1 2 3 4 9 6 6 5 3 8
1 1 2 3 4 9 6 6 5 3 8
1 1 2 3 3 9 6 6 5 4 8
1 1 2 3 3 4 6 6 5 9 8
1 1 2 3 3 4 5 6 6 9 8
1 1 2 3 3 4 5 6 6 9 8
1 1 2 3 3 4 5 6 6 9 8
1 1 2 3 3 4 5 6 6 8 9
1 1 2 3 3 4 5 6 6 8 9
1 1 2 3 3 4 5 6 6 8 9

Style

At the moment, this is C with iostream. The style obliterates the point in using C++ at all.

Code Review

Almost complete absence of standard library

The code does not leverage the functionality standard library provides. On top of that, the code is not better in any ways than standard library functionality
Using int for size of objects in memory

Indexable memory is very big nowadays. int is guaranteed to be only 16 bits, so it might not be enough to index all of the available memory on the system. Use std::size_t for that.
C style swapping

C++ has std::swap() located in <utility>. It will probably be as good as your version, if not better (it might invoke some assembly).
Using std::endl

std::endl does not only put a newline, but also flushes the buffer. Flushing is a very expensive operation, and can hinder performance of the code in tight loops.

What is done right

No using namespace std

This is good step forward. Do not use it, in any IDE/text editor code highlighting and completion will be available, if not, find a plugin for your text editor. By grabbing everything in std, you will overwhelm the ability of the IDE/text editor to give you real-time code completion suggestions.
Uniform initialization

Using uniform initialization avoids all of the vexing parse issues. Do note though, that sometimes they might invoke std::initializer_list constructor, which might not be what you need.

tl;dr

Use Standard library. Use concepts and idioms. Any decent C++ programmer knows that to reimplement standard library one must have a very good reason to do so.


Java implementation of the longest monotone increasing subsequence Problem: Given a set of n distinct numbers, find the length of the longest monotone increasing subsequence.
For example, let's take this array [1,2,9,4,7,3,11,8,14,6]
The longest monotone increasing subsequence of this array is [1,2,4,7,11,14]. So the desired result is, the length of the array which is 6.

Algorithm (taken from R.G. Dromey's How to Solve it by computer):

Establish the data set a[1...n] of n elements.
Set two loops, one to find out out each terminating point and the inner loop to find it's increasing subsequence.
start the first loop at index 2 so that it will initially have a sequence.
in the inner loop,
if current element is less than maximum of previous sequence, then,
identify the subsequence and if the length of subsequence is larger than the length of previous longest subsequence then update the length, position of maximum and maximum value.

else, update the length, position of maximum and maximum value so far.

import java.util.*;

public class Monotone {

  public static int maxMonotonicSequenceLength(int[] elements) {
    int maxValue = 0;
    int positionOfMax = 0;
    int length = 2;

    for (int i = 2; i < elements.length; ++i) {
      if (elements[i] < maxValue) {
        int count = 1;
        int previousMaxElement = 0;

        for (int j = 1; j <= i-1; ++j) {
          if ((elements[j] < elements[i]) &&
              previousMaxElement < elements[j]) {

            previousMaxElement = elements[j];
            count = count + 1;
            if (length < count) {
              length = count;
              positionOfMax = j;
              maxValue = elements[j];
            }
          }
        }
      } else {
        positionOfMax = i;
        maxValue = elements[i];
        length = length + 1;
      }
    }
    return length + 1;
  }

  public static void main(String args[]) {
    int[] list = new int[10];

    list[0] = 1;
    list[1] = 2;
    list[2] = 9;
    list[3] = 4;
    list[4] = 7;
    list[5] = 3;
    list[6] = 11;
    list[7] = 8;
    list[8] = 14;
    list[9] = 6;

    System.out.println("lenght of subsequence: " + maxMonotonicSequenceLength(list));
  }
}

Coding details

You can initialize list like this : int[] list = {1,2,9,4,7,3,11,8,14,6};
for (int j = 1, n = i-1; j <= n; ++j) { // do not loop on limit calculation
length++; // for length = length + 1;


Replace string of class of button by class so that only one button is filled in using bootstrap buttons I'm using bootstrap buttons, and in this case I have 5 different buttons with 5 different classes. When one of the buttons is clicked on, I want it to be the only one filled in, and the others to only be the outline. Here is how I currently do it:

$(".canvas_btn").on("click",function(){
  for(var i = 0; i < $(".canvas_btn").length; i++){
    document.getElementById($(".canvas_btn")[i].id).className = document.getElementById($(".canvas_btn")[i].id).className.replace("btn-outline-","btn-");
    document.getElementById($(".canvas_btn")[i].id).className = document.getElementById($(".canvas_btn")[i].id).className.replace("btn-","btn-outline-");
  }
  document.getElementById(this.id).className = document.getElementById(this.id).className.replace("btn-outline","btn");
});
<script src="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/js/bootstrap.min.js"></script>
<link href="https://stackpath.bootstrapcdn.com/bootstrap/4.1.3/css/bootstrap.min.css" rel="stylesheet"/>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<button class="btn btn-outline-info     canvas_btn" align="left"  id="new_text_btn">Text</button>
<button class="btn btn-outline-secondary  canvas_btn" align="left"  id="new_audio_btn">Audio</button>
<button class="btn btn-outline-success   canvas_btn" align="left"  id="new_image_btn">Image</button>
<button class="btn btn-outline-danger   canvas_btn" align="left"  id="new_video_btn">Video</button>
<button class="btn btn-outline-warning   canvas_btn" align="left"  id="new_input_btn">Input</button>

It feels a bit odd the way I've done it - is there a more elegant way to do it? A way to use jQuery to make it tidier would be great!

I'm also not sure how I'm causing the error in the console - I think it's to do with the bootstrap script I call in, so don't know what I can do about that.

Here's how I refactored your code:

$(document).ready(function () {

    $(".canvas_btn").on("click", function () {

        var idOfClickedElem = $(this).attr('id');
        $.each($('.canvas_btn'), function () {
            if (idOfClickedElem === $(this).attr('id')) {
                $(this).attr('class', getFilledClassName($(this).attr('class')))
            }
            else {
                $(this).attr('class', getOutlineClassName($(this).attr('class')))
            }
        });

    });

    function getOutlineClassName(currentClasses) {
        return currentClasses.replace("btn-outline-", "btn-").replace("btn-", "btn-outline-")
    }

    function getFilledClassName(currentClasses) {
        return currentClasses.replace("btn-outline", "btn")
    }

});

Instead of mixing jQuery and regular JavaScript selectors, I just used jQuery. I'm not a huge fan of the contents of getOutlineClassName(currentClasses) and getFilledClassName(currentClasses), but it's certainly less code than the way I probably would have written it if I had done so from scratch -- which would be to make a switch on the element's id and return the correct classes as a string accordingly.


Generalized Ajax function Not sure if this question will be considered "off topic". If it is, I'll remove it, but:
I hadn't see this yet so I wrote it and would like to know if this is a good approach to it. Would anyone care to offer improvements to it, or point me to an example of where someone else has already written it better?

function clwAjaxCall(path,method,data,asynch)
  {
    var xmlhttp;
    if (window.XMLHttpRequest)
      {// code for IE7+, Firefox, Chrome, Opera, Safari
      xmlhttp=new XMLHttpRequest();
      }
    else
      {// code for IE6, IE5
      xmlhttp=new ActiveXObject("Microsoft.XMLHTTP");
      }
    if(asynch)
      {
        xmlhttp.onreadystatechange=function()
          {
          if (xmlhttp.readyState==4 && xmlhttp.status==200)
            {
            //alert(xmlhttp.responseText);
            //var newaction=xmlhttp.responseText;
            //alert('Action becomes '+newaction);
            return xmlhttp.responseText;
            }
          }
      }
    if(method=='GET'){path=path+"/?"+data;}
    xmlhttp.open(method,path,asynch);
    if(method=='GET'){xmlhttp.send();}else{xmlhttp.send(data);}
    if (!asynch){return xmlhttp.responseText;}

  }

I then called it like

Just Testing
<script type="text/javascript" src="/mypath/js/clwAjaxCall.js"></script>
<script type="text/javascript">
  document.write("<br>More Testing");
  document.write(clwAjaxCall("http://www.mysite.com",'GET',"var=val",false));
</script>

UPDATE

I don't know if it's any better to anyone else than it was before, but I like it. :-)

I have re-written it like this:

// I wrote this with help from StackOverflow and have twaeked it some since then.
// call it like this:
//
//        var holder={};
//        holder.text='';
//        watch(holder,function(){ //depends on watch.js. You might choose a different watch/observe method.
//          //do stuff with holder.text in here
//        });
//        var path='http://www.example.com/?key=value';
//        AjaxCall(path,"get",null,true,holder);

function AjaxCall(path,method,data,asynch,holder){
    // holder is expected to be an object. It should be watched with watch.js, Object.observe or a similar method as they become available.
    var xmlhttp;
    if (window.XMLHttpRequest){// code for IE7+, Firefox, Chrome, Opera, Safari
      xmlhttp=new XMLHttpRequest();
    }
    else{// code for IE6, IE5
      xmlhttp=new ActiveXObject("Microsoft.XMLHTTP");
    }
    if(asynch){
      xmlhttp.onreadystatechange=function(){
        if (xmlhttp.readyState==4 && xmlhttp.status==200){
          holder.text=xmlhttp.responseText;
        }
      }
    }
    if(method=='GET'){path=path+"/?"+data;}
    xmlhttp.open(method,path,asynch);
    if(method=='GET'){xmlhttp.send();}else{xmlhttp.send(data);}
    if (!asynch){return xmlhttp.responseText;}
}

Your code seems fairly robust, but it is inefficient. Here are a few ways you could speed it up.

Reuse request objects. There is a good bit of overhead creating these, so you definitely want to be reusing these if you are making more than a few requests per page. But be careful - some older browsers have strange quirks about reused request objects.
Don't make a new function for every onreadystatechange. The function is executed with xmlhttp as the context, so, within that function, this === xmlhttp.
Whenever possible, cache. You can cache whether the browser supports XMLHTTPRequest or ActiveX so you don't check every time the function is called.
synchronous requests are usually a bad idea. Are you sure you should be using them? If not, don't write code that is more general than it needs to be. It will just make the function less efficient.

Also, you're missing a semicolon.

As an example, here's a loader function I wrote a while ago. The feature set is a little different than yours - it doesn't support synchronous requests, but it does support error handlers, data encoding, and timeouts - and behind the scenes it implements all the optimizations above. Like your code, it results in a single function; the simplest use case would be load('log.php');.

/***
* load.js
*   Simplifies use of ajax.
* parameters
*   url     : String : address of requested content
*   options : Object : (optional) additional parameters. All keys are optional.
*     onload  : Function : passed request.responseText on success
*     onfail  : Function : passed status on status!=200 or 'timeout' on timeout
*     post    : Object   : POST key-value pairs. GET is used when post==false
*     timeout : Number   : seconds to wait before aborting request
***/

var load=(function() {
    var pool=[],
        getReq=window.XMLHttpRequest?
            function() {return new XMLHttpRequest();}:
            function() {return new ActiveXObject('Microsoft.XMLHTTP');};
    function statechange() {
        if(this.readyState==4) {
            clearTimeout(this.t);
            this.onreadystatechange=null;
            pool[pool.length]=this;
            var fn=this.o[this.status==200?'onload':'onfail'];
            delete this.o;
            fn&&fn(this.status==200?this.responseText:this.status);
        }
    }
    return function(url,options) {
        var req,t,data='';
        req=pool.length?pool.pop():getReq();
        req.o=options=options||{};
        if(options.post) {
            for(t in options.post)
                data+='&'+encodeURIComponent(t)+'='+encodeURIComponent(options.post[t]);
            data=data.substring(1);
            req.open('POST',url,true);
            req.setRequestHeader('Content-type','application/x-www-form-urlencoded');
        } else {
            req.open('GET',url,true);
        }
        req.onreadystatechange=statechange;
        req.send(data||undefined);
        if(options.timeout) {
            req.t=setTimeout(function() {
                req.onreadystatechange=null;
                req.abort();
                pool[pool.length]=req;
                if(req.o.onfail)
                    req.o.onfail('timeout');
            },options.timeout*1000);
        }
    };
})();


Hide certain email characters Hide email characters:

When email only has one letter then just cover that letter with the * symbol e.g. *@gmail.com

When email has two letters then cover second letter with the symbol * e.g. t*@gmail.com

When email has three letters then cover second and third with the symbol * e.g. t**gmail.com

When email has four or more letters then show first three letters and add 4 stars at the ends - e.g.
tim****@gmail.com
tom****@gmail.com

My code works but I feel like its way too much hard-coding and can be done much cleaner
public static String hideSomeOfEmailValue(String privateEmail) {
StringBuilder stringBuilder = new StringBuilder(privateEmail);
int atSignIndex = privateEmail.indexOf("@");
String emailName = privateEmail.substring(0, atSignIndex);

if (emailName.length() == 1) {
    return stringBuilder.replace(0, emailName.length(), "*").toString();
}
if (emailName.length() > 1 && emailName.length() < 4) {
    return stringBuilder.replace(1, atSignIndex, StringUtils.repeat("*", emailName.length() - 1)).toString();
} else {
    stringBuilder = new StringBuilder(privateEmail.substring(0, 2));
    String emailProvider = privateEmail.substring(atSignIndex);
    stringBuilder.append("****").append(emailProvider);
    return stringBuilder.toString();
}

locate at what index at sign located
trim  everything until at sign which gives us name of the email only so we can easily.

Bugs:

privateEmail.substring(0, 2) needs to change to privateEmail.substring(0, 3) in the else, since end index is exclusive and we want to include 3 characters, not 2.

Method assumes that privateEmail contains a "@", and does not check if atSignIndex results in -1, resulting in a StringIndexOutOfBoundsException if there is no "@". Return privateEmail (with no hiding applied) early if you expect this method be handed any generic string which could potentially be an email, or throw an IllegalArgumentException if privateEmail is always expected to be an email. I consider the latter to be better design. Note that both these situations handle empty strings as well.

Improvements:

Use var for type inference where the type is obvious from the right side, like new StringBuilder(...), or String#substring. This is as of Java 10.

1 < emailName.length() is redundant because length of 0 and 1 are handled by earlier cases.

StringUtils.repeat can be replaced with String#repeat as of Java 11. Rids of need for external dependency here.

Instantiating a new StringBuilder in the else can be replaced with mutating the current one, namely with the delete and insert instance methods.

Move stringBuilder declaration lower, to only when it is needed.

Replace "*" with a static final variable, such that we could change it to "-" or "_" down the road and remain consistent across all the ifs and the else. This, despite being a character, will have to be a String to remain type-compatible with String#repeat and StringBuilder#replace.
Final code:

private static final String HIDE_CHAR = "*";

public static String asHiddenEmail(String privateEmail) {
    int atSignIndex = privateEmail.indexOf("@");

    if (atSignIndex == -1) {
        throw new IllegalArgumentException("`privateEmail` must be an email.");
    }

    var stringBuilder = new StringBuilder(privateEmail);
    var emailName = privateEmail.substring(0, atSignIndex);

    if (emailName.length() == 1) {
        return stringBuilder.replace(0, emailName.length(), HIDE_CHAR).toString();
    }
    if (emailName.length() < 4) {
        var hiddenNameEnd = HIDE_CHAR.repeat(emailName.length() - 1);
        return stringBuilder.replace(1, atSignIndex, hiddenNameEnd).toString();
    }
    else {
        stringBuilder.delete(3, atSignIndex);
        stringBuilder.insert(3, HIDE_CHAR.repeat(4));
        return stringBuilder.toString();
    }
}


Currying addition function in ECMAScript 6 I wanted to write an add function that implemented currying in JavaScript. I welcome criticism and suggestions for improvement.

function add(...args1) {
  if(!args1.length) {
    return sum(args1);
  }

  return (...args2) => {
    if(!args2.length) {
      return sum(args1.concat(args2));
    }

    return add.apply(null, args1.concat(args2));
  };
}

function sum(arr) {
  return arr.reduce((p,c) => p+c,0);
}

console.assert(add(1,2)(3)(4)() === 10, '1+2+3+4 should be 10');

I'm unsure about this part:

if(!args1.length) {
  return sum(args1);
}

If the array's empty, you could just return zero - no need to call sum.

Secondly, you're doubling possible return types for add. You either get a new curried function, or you get a number.

This goes for the curried function as well, really, as it too will return either a sum or another function.

So either way, it's kinda tricky. Partial application/currying is kinda difficult if you have something like this, that you can "keep currying" forever.

Anyway, sticking to your current structure, it can be simplified somewhat. Notably, you don't have to create a new closure for each invocation. And you can get rid of the double !argsN.length checks and calls to sum:

function add(...initial) {
  let list = [];

  function adder(...values) {
    if(!values.length) {
      return list.reduce((sum, n) => sum + n, 0);
    }

    list.push(...values);
    return adder;
  }

  return adder(...initial);
}

Here, I've named the nested function, so it can return itself. After the initial call to add, the nested adder function is created, closes over the list array, and is invoked. It then either returns the sum or it returns itself. I.e. only one closure is created, and subsequent calls simply update the closed over list - or sums it.

adder also handles all the input checking etc.. The outer add function only exists to create and invoke it.

Also note that I renamed the argument arrays, as args1 and args2 really aren't too descriptive. And that the ...-splat works for both definition and invocation. And, lastly, since it only need to do a sum in 1 place, there's no need for a separate sum function.


A simple Ruby EmailAddress class I wrote this short, plain Ruby lib to handle email address validation inside and outside of Rails applications, and I would like to know what you think.

require "i18n"

class EmailAddress
  VALID_EMAIL_ADDRESS_REGEX = /\A[\w+\-.]+@[a-z\d\-.]+\.[a-z]+\z/i

  def self.malformed?(email)
    email !~ VALID_EMAIL_ADDRESS_REGEX
  end

  def self.valid?(email)
    !malformed?(email)
  end

  attr_reader :email, :i18n_scope, :errors

  def initialize(email, i18n_scope: [:models, :email_address])
    @email = email
    @i18n_scope = i18n_scope
    @errors = []
  end

  def valid?
    validate
    errors.empty?
  end

  def validate
    message = I18n.t(:invalid, scope: i18n_scope)
    errors.push(message) if EmailAddress.malformed?(email)
  end
end

Usage can be as simple as:

if EmailAddress.valid?(params[:email))
  # do something
end

Or for remote validations, for example:

def email
  email = EmailAddress.new(params[:email])
  if email.valid?
    render nothing: true, status: 200
  else
    render json: { errors: email.errors }, status: 422
  end
end

The Regex will validate some email addresses. Because the Domain Name System allows UniCode. the Regex does not match all valid characters in the Doman Name. Therefore it will not validate all valid email addresses.
Alternatives:
Validating over all allowable Unicode with Regex in a meaningful way is non-trivial.

Sending a confirmation email requiring reply is a straight-forward method for validating email procedurally.

When possible, not collecting email addresses at all is easy to implement.


Finding which items have at least one common value with other items I have the following collection :

var items = new[]
{
    new { Name = "A", Values = new [] { 20, 4, 5, 9, 3, 22 } },
    new { Name = "B", Values = new [] { 10, 7, 9, 8, 42 } },
    new { Name = "C", Values = new [] { 11, 103, 0 } },
    new { Name = "D", Values = new [] { 7, 35, 42 } }
};

I would like to find which items have at least one common integer value with the others items. For example, "A" has 9 in common with "B", so "A" and "B" are related each other.

At the end, this should be the result :

A => [B]
B => [A, D]
C => []
D => [B]

I wrote this code :

var tuples = items
    .SelectMany(x => x.Values, (x, y) => new { x.Name, Value = y })
    .ToList();

var result = tuples.Join(tuples,
           x => x.Value,
           x => x.Value,
           (x, y) => new { NameA = x.Name, NameB = y.Name })
    .Distinct()
    .GroupBy(x => x.NameA, x => x.NameB,
        (a, b) => new { Name = a, Overlaps = b.Where(x => x != a)) });

It works (and return expected results). I was wondering if there would be a more efficient (faster and with a lower memory footprint) way do to it. I wanted to use GroupJoin() but since I need to filter out duplicate values (using Distinct) I used Join() instead.

For set-based operations like this it's better to use HashSet<T>.

  The HashSet<T> class provides high-performance set operations.

The items can easily be converted to ones containing HashSets. Then, the Overlaps method does the comparison:

var hashed = items
    .Select(i => new { Name = i.Name, Values = i.Values.ToHashSet() })
    .ToList();
var overlaps = hashed.Select(h1 => new
{
    h1.Name,
    Overlaps = hashed
        .Where(h2 => h2.Name != h1.Name && h2.Values.Overlaps(h1.Values))
        .Select(h => h.Name)
});

Result:

A => B
B => A,D
C
D => B

Of course, if possible, it would be better to create items with HashSets at the outset.

EDIT

Triggered by your comment I did some benchmarking. I blew up your array a 1000 times, keeping unique names and then just measured both methods, using Linqpad:

var items = Enumerable.Range(1,1000).SelectMany(e =>  new[]
{
    new { Name = "A" + e, Values = new [] { 20, 4, 5, 9, 3, 22 } },
    new { Name = "B" + e, Values = new [] { 10, 7, 9, 8, 42 } },
    new { Name = "C" + e, Values = new [] { 11, 103, 0 } },
    new { Name = "D" + e, Values = new [] { 7, 35, 42 } }
});

var sw = Stopwatch.StartNew();
var hashed = items.Select(i => new { Name = i.Name, Values = i.Values.ToHashSet() }).ToList();

sw.Elapsed.Dump();

var overlaps = hashed.Select(h1 => new
{
    h1.Name,
    Overlaps = hashed
        .Where(h2 => h2.Name != h1.Name && h2.Values.Overlaps(h1.Values))
        .Select(h => h.Name).ToList()
}).ToList();
sw.Stop();
sw.Elapsed.Dump();

sw.Restart();
var tuples = items
    .SelectMany(x => x.Values, (x, y) => new { x.Name, Value = y })
    .ToList();

sw.Elapsed.Dump();
var result = tuples.Join(tuples,
           x => x.Value,
           x => x.Value,
           (x, y) => new { NameA = x.Name, NameB = y.Name })
    .Where(x => x.NameA != x.NameB)
    .Distinct()
    .GroupBy(x => x.NameA, x => x.NameB)
    .ToList();
sw.Stop();
sw.Elapsed.Dump();

The results:

00:00:00.0035664
00:00:02.0326684
00:00:00.0029370
00:00:06.3872002

As you see, the preparatory actions for both methods are done in 'no time', but the hashset-based method is considerably faster. Interestingly, if I blow up the arry 2000 times, the hashset-based method takes 7.7s and your method throws an OutOfMemoryException on my box.

Side note: if I don't make the names unique both methods are comparable. (On which I based my previous comment).


Print fragment of multiplication table I am learning Python online on Stepik platform, and unfortunately I don't have any opportunity to get reviews from the tutor. The platform only checks the correctness of the answer that my program gives. So I decided to share the code here. Comments and corrections would be much appreciated.

  Write a program that print multiplication table
  fragment for all numbers in the interval [a; b] for all of the
  interval [c; d]

def print_mult_table(a, b, c, d):
    """
    Print multiplication table fragment for all numbers in the interval
    [:param a; :param b] for all of the interval [:param c; :param d]
    """
    m = list(list([i] + list(range(c * i, (d + 1) * i, i))) for i in ([1] + list(range(a, b + 1))))
    m[0][0] = ""    # This is done, because it is the desired output format
    max_width = len(str(m[-1][-1])) + 1
    for i in m:
        i = [str(j).rjust(max_width) for j in i]
        print('\t'.join(i))

I would make also this function a generator and make printing the responsibility of the caller. This makes the function more reusable.

def print_mult_table(row_start, row_end, col_start, col_end):
    """
    Print multiplication table fragment for all numbers in the interval
    [:param row_start; :param row_end] for all of the interval [:param col_start; :param col_end]
    """
    table = list(list([row] + list(range(col_start * row, (col_end + 1) * row, row))) for row in ([1] + list(range(row_start, row_end + 1))))
    table[0][0] = ""    # This is done, because it is the desired output format
    max_width = len(str(table[-1][-1])) + 1
    for row in table:
        yield '\t'.join(str(col).rjust(max_width) for col in row)

for row in print_mult_table(1, 10, 4, 8):
    print(row)

Note that I gave the variables easier to understand names. This makes the list comprehension longer. Which leads to the next point:

While the list comprehensions are nice and compact (with short incomprehensible variable names), they are starting to stretch what is still easily readable. A general rule is, if your list comprehension is longer than a line (80 characters) it is probably too complicated.  Therefore I would unfold it:

from itertools import chain

def get_mult_table(row_start, row_end, col_start, col_end):
    """
    Yield multiplication table fragment for all numbers in the interval
    [:param row_start; :param row_end] for all of the interval
    [:param col_start; :param col_end]
    """
    for row in chain([1], range(row_start, row_end + 1)):
        yield [col * row for col in chain([1], range(col_start, col_end + 1))]

def print_mult_table(row_start, row_end, col_start, col_end):
    table = list(get_mult_table(row_start, row_end, col_start, col_end))
    table[0][0] = ""    # This is done, because it is the desired output format
    max_width = len(str(table[-1][-1])) + 1
    for row in table:
        print '\t'.join(str(col).rjust(max_width) for col in row)

if __name__ == '__main__':
    print_mult_table(1, 10, 4, 8)

This makes it a lot more obvious what happens (because it includes the actual multiplication, it is not hidden in the step size of the inner range). This might make it slightly slower (when generating multiplication tables from, say, 1 to 1,000,000).

It also separates the generation of the table from the actual printing logic.


Google Foobar Challenge: Spy Snippets in Python I am getting all the answers correct. But still the solution is not accepted as only 4/5 tests cases are passed.

I have not posted the whole problem statement but the problem is similar to this.

I want to know if there are any more optimizations possible.

import sys

class Queue(object):
    input_array = []

    def __init__(self, input_array=None):
        if not input_array:
            self.input_array = []
        else:
            self.input_array = input_array

    def enqueue(self, element):
        self.input_array.append(element)

    def dequeue(self):
        return self.input_array.pop(0)

    def first(self):
        return self.input_array[0]

    def last(self):
        return self.input_array[-1]

    def size(self):
        return len(self.input_array)

    def get_queue(self):
        return self.input_array

    def get_queue_after_first(self):
        return self.input_array[1:]

    def __str__(self):
        return "Current Queue: {0}".format(self.input_array)

def answer(document, searchTerms):
    no_of_search_terms = 0
    count = dict()
    for searchTerm in searchTerms:
        if searchTerm in count:
            count[searchTerm] += 1
        else:
            no_of_search_terms += 1
            count.update({searchTerm: 1})

    q = Queue()
    len_q = Queue()
    smallest_snippet_size = sys.maxint
    offsets = tuple()
    tokens = document.split()

    for position, token in enumerate(tokens, start=1):
        if count.get(token, 0):
            q.enqueue(token)
            len_q.enqueue(position)
            while q.first() in q.get_queue_after_first():
                q.dequeue()
                len_q.dequeue()
            current_block_len = len_q.last() - len_q.first() + 1
            if (q.size() >= no_of_search_terms) and (current_block_len < smallest_snippet_size):
                smallest_snippet_size = current_block_len
                offsets = (len_q.first() - 1, len_q.last())

    return " ".join(tokens[offsets[0]: offsets[1]])

if __name__ == '__main__':
    assert (answer("world there hello hello where world", ["hello", "world"]) == 'world there hello')
    assert (answer("many google employees can program", ["google", "program"]) == 'google employees can program')
    assert (answer("some tesla cars can autopilot", ["tesla", "autopilot"]) == 'tesla cars can autopilot')
    assert (answer("a b c d a", ["c", "d", "a"]) == 'c d a')
    assert (answer("the cats run very fast in the rain", ["cats", "run", "rain"]) == 'cats run very fast in the rain')
    assert (answer("the cats run very fast in the rain run cats", ["cats", "run", "rain"]) == 'rain run cats')
    assert (answer("hello", ["hello"]) == 'hello')

Class attribute

Having input_array = [] defined at class level does not add anything except confusion.

Counter

I think that this:

no_of_search_terms = 0
count = dict()
for searchTerm in searchTerms:
    if searchTerm in count:
        count[searchTerm] += 1
    else:
        no_of_search_terms += 1
        count.update({searchTerm: 1})

could be done in a clearer and more efficient way using: collections.Counter.


Random quote generator I just began learning programming and I'm learning Objective-C. I'm taking an online course and in that course we were doing a magic crystal ball app. Then I decided to use what I learned and make my own app.

Viewcontroller .h

#import <UIKit/UIKit.h>

@class MBFQuotes;

@interface MBFViewController : UIViewController

@property (strong, nonatomic) MBFQuotes *quotes;

@property (strong, nonatomic) IBOutlet UILabel *quoteLabel;

@property (strong, nonatomic) IBOutlet UIButton *generalButton;
@property (strong, nonatomic) IBOutlet UIButton *lifeButton;
@property (strong, nonatomic) IBOutlet UIButton *politicsButton;
@property (strong, nonatomic) IBOutlet UIButton *smartButton;

//Action

- (IBAction)generalButton:(id)sender;
- (IBAction)lifeButton:(id)sender;
- (IBAction)politicsButton:(id)sender;
- (IBAction)smartButton:(id)sender;

@end

Viewcontroller .m

#import "MBFViewController.h"
#import "MBFQuotes.h"

@interface MBFViewController ()

@end

@implementation MBFViewController

- (void)viewDidLoad
{
[super viewDidLoad];
self.quotes = [[MBFQuotes alloc] init];
}

- (void)didReceiveMemoryWarning
{
[super didReceiveMemoryWarning];
// Dispose of any resources that can be recreated.
}

- (IBAction)generalButton:(id)sender {
self.quoteLabel.text = [_quotes randomGeneral];
self.quoteLabel.backgroundColor = [UIColor redColor];

}

- (IBAction)lifeButton:(id)sender {
self.quoteLabel.text = [_quotes randomLife];
self.quoteLabel.backgroundColor = [UIColor blueColor];

}

- (IBAction)politicsButton:(id)sender {
self.quoteLabel.text = [_quotes randomPolitics];
self.quoteLabel.backgroundColor = [UIColor blackColor];

}

- (IBAction)smartButton:(id)sender {
self.quoteLabel.text = [_quotes randomSmart];
self.quoteLabel.backgroundColor = [UIColor orangeColor];

}
@end

Class Quotes .h

#import <Foundation/Foundation.h>

@interface MBFQuotes : NSObject

-(NSString *)randomGeneral;
-(NSString *) randomLife;
-(NSString *) randomPolitics;
-(NSString *) randomSmart;
@end

Class Quotes.m

#import "MBFQuotes.h"

@implementation MBFQuotes

-(NSString *) randomGeneral {

 NSArray *generalQuotes = @[@"g1", @"g2", @"g3", @"g4"];

int randomGeneral = arc4random_uniform([generalQuotes count]);

return [generalQuotes objectAtIndex:randomGeneral];
}

-(NSString *) randomLife {

NSArray *lifeQuotes = @[@"l1", @"l2"];

int randomLife = arc4random_uniform([lifeQuotes count]);

 return [lifeQuotes objectAtIndex:randomLife];
}

-(NSString *) randomPolitics {

NSArray *politicsQuotes = @[@"p1", @"p2"];

int randomPolitics = arc4random_uniform([politicsQuotes count]);

return [politicsQuotes objectAtIndex:randomPolitics];
}

-(NSString *) randomSmart {

 NSArray *smartQuotes = @[@"s1", @"s2"];

int randomSmart = arc4random_uniform([smartQuotes count]);

return [smartQuotes objectAtIndex:randomSmart];
 }

 @end

The app works perfectly, but I just want to see if there is better or easier way to do it, or is there there a better design.

ViewController.h
First, this name isn't really descriptive at all, is it?  We need to know more about this view controller.  In fact, standard naming convention is to suffix all view controller subclasses with ViewController anyway, so this is basically an unnamed view controller.
Now, basically everything declared in your .h should be in the .m file.  Of the 6 property declarations, 5 of them are IBOutlet properties, and there's never really an excuse for a view controller to expose its subviews publicly.  A view controller should be in complete controller of its view.  If an outside class needs to do something that would effect one of these views, create a public method, and let that method internally handle the effects on those views.
As for the 4 IBAction method declarations, you don't even need to declare these.  The only reason to put a method in an @interface section is to expose it to another class.  And IBAction methods should never be exposed to another class for the same point as exposing IBOutlets to outside classes.  If you need to, create a public method and have that method call the IBAction method internally.
Finally, the MBFQuotes object.  Does this need public visibility?  Are you modifying it from outside classes?  Looks like probably not, and as such, we can get rid of the class declaration (@class MBFQuotes) and move the @property declaration in the the @interface section of the .m, which comes after the #import of that classes file, and eliminates the need for declaring the class.

ViewController.m
Seeing as we're not actually doing anything in didReceiveMemoryWarning, we can simply eliminate that entire method from this file.  All we're doing is calling the super method, and simply leaving the entire method out handles that.  I understand that didReceiveMemoryWarning's stub is generated automatically by Xcode when you create a new UIViewController subclass, but you should still take it out unless you intend to do something with it (and in most cases, you won't need this method).
Our four IBAction methods look quite similar, don't they?  Why don't we combine them into one method.
First, we need an enum for some readability, so at the top of this file, right after the #import statements, you'll want this code:
typedef NS_ENUM(NSInteger, QuoteTypes) {
    GeneralQuotes = 100,
    LifeQuotes = 101,
    PoliticalQuotes = 102,
    SmartQuotes = 103
};

Now, let's use the .tag property of UIButton.  You can set this on the storyboard or programmatically.  If you set it on the storyboard, set the tag for each button to match the appropriate number in the enum we just created.  Programmatically, you can set them as such:
self.generalButton.tag = GeneralQuotes;
self.lifeButton.tag = LifeQuotes;
self.politicalButton.tag = PoliticalQuotes;
self.smartButton.tag = SmartQuotes;

Now then, get rid of the 4 IBAction methods (be sure to unlink the buttons in storyboard), and create on method to handle all 4 buttons, link all of the buttons to that method.  The method should look something like this:
- (IBAction)quoteButtonPushed:(id)sender {
    switch(sender.tag) {
        case GeneralQuotes:
            self.quoteLabel.text = [self.quotes randomGeneral];
            self.quoteLabel.backgroundColor = [UIColor redColor];
            break;
        case LifeQuotes:
            self.quoteLabel.text = [self.quotes randomLife];
            self.quoteLabel.backgroundColor = [UIColor blueColor];
            break;
        case PoliticalQuotes:
            self.quoteLabel.text = [self.quotes randomPolitical];
            self.quoteLabel.backgroundColor = [UIColor blackColor];
            break;
        case SmartQuotes:
            self.quoteLabel.text = [self.quotes randomSmart];
            self.quoteLabel.backgroundColor = [UIColor orangeColor];
            break;
        default: break;
    }
}

And please notice that I've used self.quotes here rather than _quotes as you used.  If you're declaring a @property, you should be accessing that property through the accessor except in init, dealloc, and the accessor methods, should you choose to write custom accessor methods.  If you don't want to access the property through the accessors, then maybe you don't need a property and instead just need a regular instance variable.

MBFQuotes.h / MBFQuotes.m
As it stands, this is really incomplete as a class, and doesn't really justify being a class at all.  It could easily be simply a set of C-style functions.  Which way do you want to go?  Right now, you're in between.
If we're happy with the methods as they are and don't want to add anything, then let's change this to some C-style functions:
// MBFQuotes.h

#import <Foundation/Foundation.h>

NSString * randomGeneral();
NSString * randomLife();
NSString * randomPolitical();
NSString * randomSmart();

Notice the slightly different style and the elimination of @interface and @end.  This is no longer a class, but instead a header file with some C-style function declarations.
EDIT: All right, so you can't actually have const NSArray objects as far as I can tell.  The answer has been edited to the next best solution:
// MBFQuotes.m

#import "MBFQuotes.h"

NSString * randomGeneral() {
    static NSArray *generalQuotes = @[@"g1", @"g2", @"g3", @"g4"];
    int randomGeneral = arc4random_uniform([generalQuotes count]);
    return [generalQuotes objectAtIndex:randomGeneral];
}

NSString * randomLife() {
    static NSArray *lifeQuotes = @[@"l1", @"l2"];
    int randomLife = arc4random_uniform([lifeQuotes count]);
    return [lifeQuotes objectAtIndex:randomLife];
}

NSString * randomPolitical() {
    static NSArray *politicsQuotes = @[@"p1", @"p2"];
    int randomPolitical = arc4random_uniform([politicsQuotes count]);
    return [politicsQuotes objectAtIndex:randomPolitical];
}

NSString * randomSmart() {
    static NSArray *smartQuotes = @[@"s1", @"s2"];
    int randomSmart = arc4random_uniform([smartQuotes count]);
    return [smartQuotes objectAtIndex:randomSmart];
}

Notice moving the array declaration out of the functions/methods and declaring them as constants.  This makes the program a lot more efficient.
Anyway, if you don't want to go this route, there is certainly room to turn this into a class.  For starters, you'd still want to move the arrays out of the methods, but now you'll want them as private properties.  For this to be truly classworthy though, you'd want to include methods to be able to add and remove quotes, perhaps give the class a file-path and parse quotes out of a text file.
Realistically, since this is a class and we're instantiating it, we shouldn't have different categories of quotes.  Realistically, we should instantiate a different object of this class for each quote category.
UNLESS... we were to add a method to pick a random quote from ALL categories (single method, random category, random quote).

Finally, I think for the project to be really complete, instead of the MBHQuotes class simply holding an array of strings, completeness would say there should be a Quote class, which consists of a few properties.  The main property is the quote itself.  The other properties give some information about the quote, such as the author, the date of the quote's origin, the category of quote it fits into, etc.


Scraping weather conditions for mountain peeks I have built a website in Django in which I tried for the first time to create a complex app. It's an app that scrapes weather conditions on a peak in mountains, which are 17. I wanted to show a detailed forecast in separate templates, so I have 17 views which look almost the same.
Only 4 views:
class KasprowyWierchForecastView(TemplateView):
    """
    class for rendering view of detailed weather forecast for
    Kasprowy Wierch peak
    """
    template_name = 'mountain_base.html'

    def get_context_data(self, **kwargs):
        context = super().get_context_data(**kwargs)
        context['Peak'] = PeakForecast.objects.filter(
            name_of_peak='Kasprowy').order_by('date')
        print(context['Peak'])
        return context

class KoscielecForecastView(TemplateView):
    """
    class for rendering view of detailed weather forecast for
        Koscielec peak
    """
    template_name = 'mountain_base.html'

    def get_context_data(self, **kwargs):
        context = super().get_context_data(**kwargs)
        context['Peak'] = PeakForecast.objects.filter(
            name_of_peak='Koscielec').order_by('date')
        return context

class KrivanForecastView(TemplateView):
    """
    class for rendering view of detailed weather forecast for Krivan peak
    """
    template_name = 'mountain_base.html'

    def get_context_data(self, **kwargs):
        context = super().get_context_data(**kwargs)
        context['Peak'] = PeakForecast.objects.filter(
            name_of_peak='Krivan').order_by('date')
        return context

class MieguszowieckiForecastView(TemplateView):
    """
    class for rendering view of detailed weather forecast for
    Mieguszowiecki peak
    """
    template_name = 'mountain_base.html'

    def get_context_data(self, **kwargs):
        context = super().get_context_data(**kwargs)
        context['Peak'] = PeakForecast.objects.filter(
            name_of_peak='Mieguszowiecki').order_by('date')
        return context

class MnichForecastView(TemplateView):
    """
    class for rendering view of detailed weather forecast for
    Mnich peak
    """
    template_name = 'mountain_base.html'

    def get_context_data(self, **kwargs):
        context = super().get_context_data(**kwargs)
        context['Peak'] = PeakForecast.objects.filter(
            name_of_peak='Mnich').order_by('date')
        return context

Only the name of the view class and context['Peak'] are different. The rest of the code is redundant. As my experience is based only on my self learning process I've not found any nice solution on how to fix or refactor.

One option is to modify your TemplateView class to include the peak's name  as an instance attribute (say peak_name) and make the get_context_data method make use of it:
class  TemplateView():
    # Class attributes

    def __init__(self, peak_name):
        # Instance attributes
        # ...
        self.peak_name = peak_name

    # Class methods

    # Modification of the get_context_data method
    def get_context_data(self, **kwargs):
        context['Peak'] = PeakForecast.objects.filter(
            name_of_peak=self.peak_name).order_by('date')  #<- Modification here
        print(context['Peak'])
        return context

And then you can generate your peak views as instances of that class:
krivan_view = TemplateView('Krivan')
krivan_view.get_context_data(arg1='arg1', arg2='arg2')


Palindrome Checker Algorithm Here is an example of how I solved this problem (though this is not the only nor the best way by any means) for an assignment where it was also requisite to determine if a string was a reverse prefix of the other. Obviously a palindrome is a reverse prefix of itself so if you can determine a reverse prefix then you can determine a palindrome.

I would love some feedback on my code as well as any thoughts on my implementation/solution.

int main()
{
    char string1[50];
    char string2[50];

    printf("Please enter a string: ");
    gets(string1);
    if (strlen(string1) == 1)
        exit(1);
    printf("Please enter a second string: ");
    gets(string2);
    printf("\nString 1 = %s", string1);
    printf("\nString 2 = %s\n", string2);
    if (isReversePrefix(string1, string2))
        printf("String 2 is a reversed prefix of string 1\n");
    if (isPalindrome(string1))
        printf("String 1 is a palindrome\n");
    else
        printf("String 1 is not a palindrome\n");
    if (isPalindrome(string2))
        printf("String 2 is a palindrome\n");
    else
        printf("String 2 is not a palindrome\n");
    return 0;
}
/************************************************
 isReversePrefix - Receives two string pointers
 and determines if the second is a reverse
 prefix of the first. Returns either true or
 false.
 The word 'HIM' has the following prefixes 'H', 'HI',
 'HIM'. All of these can be reversed to 'H', 'IH', 'MIH'
 ************************************************/
int isReversePrefix(char *str1, char *str2)
{
    char *temp1 = str1;
    char *temp2 = str2;

    if (strlen(str2) > strlen(str1))
        return 0;

    strrev(temp2);
    while ((*temp1 == *temp2) && (*temp2 != '\0'))
    {
        temp1++;
        temp2++;
    }
    if (*temp2 == '\0')
        return 1;
    else
        return 0;
}
/************************************************
 isPalindrome - receives one string parameter and
 checks if it is a palindrome or not. Returns
 true if it is and false if not.
************************************************/
int isPalindrome(char *str1)
{
    char *start = str1;
    char *end = str1 + strlen(str1) - 1;
    if (isReversePrefix(start, end))
        return 1;
    else
        return 0;
}

Since the char array sizes are fixed, you should make sure the inputs are of an appropriate size before continuing.  This should best be done with the function doing the reading.

Very important: gets() is unsafe as it has no knowledge of buffer size thus is vulnerable to buffer overflow.  Instead, use fgets(), which is safer as it takes a buffer size as an argument.
Consider hard-coding the 50 to make its intent clear.

#define MAX_LENGTH 50

Since you're already in main(), you can use return 1 instead of exit(1).
Unformatted strings like this:

printf("Please enter a string: ");

can instead be output using puts():

puts("Please enter a string: ");

Be aware that puts() automatically appends a newline at the end.  If you don't want this newline, then you can use fputs() instead:

fputs("Please enter a string: ", stdout);


Wikipedia search using the JSON API What could I do better in my code or logic?

Wikipedia Search

$(function(){

  $('.search-input').keyup(function(data){

  var pesquisar = $('.search-input').val();

  $('.box-logo').fadeOut(500, function(){
    $('header').css('padding','12px 0');
  });

$.getJSON('https://en.wikipedia.org/w/api.php?format=json&action=query&generator=search&gsrnamespace=0&gsrlimit=10&prop=pageimages|extracts&pilimit=max&exintro&explaintext&exsentences=1&exlimit=max&gsrsearch='+ pesquisar +'&callback=?',function(data){

  console.log(data);

  var key;

  $('.sec-result .row').html('');
  for(key in data.query.pages){

    var titleArt = data.query.pages[key].title;
    var extractArt = data.query.pages[key].extract;
    var linkArt = 'https://en.wikipedia.org/?curid=' + data.query.pages[key].pageid;
    var imgArt;

    if(data.query.pages[key].hasOwnProperty('thumbnail')){
      imgArt = data.query.pages[key].thumbnail.source;
    } else {
      imgArt = 'http://www.wallpaperup.com/uploads/wallpapers/2014/04/02/319530/big_thumb_e96d0c33f97706bc093572bc613cb23d.jpg';
    }

    var contentHTML = '

'+ titleArt +'

' + extractArt + '

';

    $('.sec-result .row').append(contentHTML);
  }

 });

 })

})

I would use map to replace all the data.query.pages[key]. into a single variable. Also ES6 template string is very useful, if you are not using ES6, just change it into one line string like you did in your original solution. map return an array, so I have to join('') them into a single string. Then I can just use it to pass to the jquery html() function and replace the content. Also, ? : shorthand for if else statement can produce less code in one line.

$('.sec-result .row').html(
    data.query.pages.map(page => (
        `

${page.title}

${page.extract}

`
    )).join('')
);

My solution is just for readability, it may not increase the speed though.


Dijkstra-like routing algorithm I've got the following code to find the shortest path between two nodes using almost-Dijkstra's-algorithm which was written for an exercise, and I'm looking for things to improve my general Scala style. What I currently have:

  object Graphs {
    case class Node[T](value : T)
    case class Edge[T](from : Node[T], to : Node[T], dist : Int)

    def shortest[T](edges : Set[Edge[T]], start : T, end : T) : Option[List[Edge[T]]] = {

      val tentative = edges.flatMap(e => Set(e.from, e.to)).map(n => (n, if (n.value == start)  Some(List.empty[Edge[T]]) else None )).toMap

      def rec(tentative : Map[Node[T], Option[List[Edge[T]]]]) : Option[List[Edge[T]]] = {
        val current = tentative.collect{ case (node, Some(route)) => (node, route)}.toList.sortBy(_._2.length).headOption

        current match {
          case None => None
          case Some((node, route)) => {
            if (node.value == end) Some(route)
            else {
              val fromHere = edges.filter(e => e.from == node)
              val tentupdates = for(edge <- fromHere if tentative.contains(edge.to)) yield {
                tentative.get(edge.to) match {
                  case None => throw new Error("broken algorithm")
                  case Some(Some(knownroute)) if (knownroute.map(_.dist).sum < route.map(_.dist).sum + edge.dist) => (edge.to, Some(knownroute))
                  case _ =>(edge.to, Some(edge :: route))
                }
              }

              val newtentative = (tentative ++ tentupdates) - node
              rec(newtentative)
            }
          }
        }
      }
      rec(tentative)
    }
  }

First of, getting some feedback on the correctness would be nice.

For the algorithm itself, I already know it could be refined by keeping track of the unevaluated edges, and for a more general solution keeping a second accumulator with the solved set wouldn't cost me much more, but I get the general idea of how to implement that.

I'm thinking of replacing Node with Scalaz TypeTags, and would like some feedback on whether that's a good idea.

Other than that, I'd like some feedback on general style, and how to improve readability - I have quite long lines now for example. Also, types like Map[Node[T], Option[List[Edge[T]]]] sort of hurt my eyes, I'd love to know how I could improve that.

Lastly, I really don't like my

case Some(Some(knownroute)) if (knownroute.map(_.dist).sum < route.map(_.dist).sum + edge.dist) => (edge.to, Some(knownroute))
case _ => (edge.to, Some(edge :: route))

the first case is only used as a filter, I'm actually looking for something like

case None || Some(Some(knownroute)) if (knownroute.map(_.dist).sum > route.map(_.dist).sum + edge.dist) => (edge.to, Some(edge :: route))

but I don't know how to express that.

There are a few things how this could be improved.

First, a few minor quibbles about syntax:

When using a type annotation, do not put a space before the : – name: Type, please. (source)
In a chain of higher-order methods, do not use the . to invoke the method. For example this

val tentative = edges.flatMap(e => Set(e.from, e.to)).map(n => (n, if (n.value == start)  Some(List.empty[Edge[T]]) else None )).toMap

should be

val tentative = edges flatMap (e => Set(e.from, e.to)) map (n => (n, if (n.value == start) Some(List.empty[Edge[T]]) else None)).toMap

(source)
When a chain of transformations is very long, splitting inside the lambdas can be an acceptable solution:

val tentative = edges flatMap (
    e => Set(e.from, e.to)
  ) map (n =>
    (n, if (n.value == start) Some(List.empty[Edge[T]]) else None)
  ).toMap

yield { block } is “evil” and should be avoided. for-comprehensions can be rewritten less clearly with flatMap and filter, but this may actually be preferable when the transformations are deeply nested.

Now let's look at this piece of your code:

for(edge <- fromHere if tentative.contains(edge.to)) yield {
  tentative.get(edge.to) match {
    case None => throw new Error("broken algorithm")
    case Some(Some(knownroute)) if (knownroute.map(_.dist).sum < route.map(_.dist).sum + edge.dist) => (edge.to, Some(knownroute))
    case _ =>(edge.to, Some(edge :: route))
  }
}

As I said, this could be rewritten to avoid the comprehension.

fromHere filter (edge => tentative.contains(edge.to)) flatMap { edge =>
  tentative.get(edge.to) match {
    case None => throw new Error("broken algorithm")
    case Some(Some(knownroute)) if (knownroute.map(_.dist).sum < route.map(_.dist).sum + edge.dist) => (edge.to, Some(knownroute))
    case _ => (edge.to, Some(edge :: route))
  }
}

The tentative.get(…) returns an Option, which will be None if no element for that key was found. But that means we can get rid of the filter! Instead, we map over the result of the get, which removes one level of Options:

fromHere flatMap { edge =>
  tentative.get(edge.to) map {
    case Some(knownroute) if (knownroute.map(_.dist).sum < route.map(_.dist).sum + edge.dist) => (edge.to, Some(knownroute))
    case _ => (edge.to, Some(edge :: route))
  }
}

Destructuring the Option with Pattern matching is a bit tedious. Actually, we want to do one thing, orElse some default case.

fromHere flatMap { edge =>
  tentative.get(edge.to) map { maybeRoute =>
    maybeRoute filter (knownroute =>
      knownroute.map(_.dist).sum < route.map(_.dist).sum + edge.dist
    ) map (knownroute =>
      Pair(edge.to, Some(knownroute)
    ) getOrElse (Pair(edge.to, Some(edge :: route)))
  }
}

But this is an unreadable mess! Yes, it somehow is. We can improve this by adding a Route class, e.g:

case class Route[T](route: List[Edge[T]], dist: Int) {
  val length = route.length

  def this(route: List[Edge[T]]) = this(route, route map (_.dist) sum)

  def this() = this(List.empty[Edge[T]], 0)

  def ::(edge: Edge[T]) = Route(edge :: route, dist + edge.dist)
}

val tentative: Map[Node[T], Option[Route[T]] = ...

The main advantage is that this keeps track of a route's distance, which means the above code becomes the slightly more accessible

fromHere flatMap { edge =>
  tentative.get(edge.to) map { maybeRoute =>
    val maybeBetterRoute =
      maybeRoute filter (knownRoute => knownRoute.dist < route.dist + edge.dist)
    maybeBetterRoute map (knownRoute =>
      Pair(edge.to, Some(knownroute)
    ) getOrElse (
      Pair(edge.to, Some(edge :: route))
    )
  }
}

You calculate the fromHere each time, which is an O(n) calculation, I think:

val fromHere = edges.filter(e => e.from == node)

It may be better to build a Map[Node[T], List[Edge[T]]] before the recursion, which is also a fairly cheap operation:

val edgesBySource = edges groupBy (_.from)

then: val fromHere = edgesBySource.get(node).flatten. I assume this would pay off soon for non-tiny graphs.


Prolog Sudoku solver taking too long I was wondering if there's any way I can improve this code's execution.  I want to use it on a 9*9 grid, but it takes too long to solve this as 4*4.  This program takes as input a 4*4 matrix represented as a 1D matrix and returns the solution to the puzzle in the variable solution.  Any comments?

subset([],_).
subset([H|T],List):-
    !,
    member(H,List),
    subset(T,List).

different([]).
different([H|T]):-
\+member(H,T),
different(T),!.

valid([]).
valid([Head|Tail]) :-
    different(Head),
    valid(Tail),!.

sudoku(Puzzle, Solution) :-
    Solution = Puzzle,
    Puzzle = [S11, S12, S13, S14,
              S21, S22, S23, S24,
              S31, S32, S33, S34,
              S41, S42, S43, S44],

    subset(Solution, [1,2,3,4]),

    Row1 = [S11, S12, S13, S14],
    Row2 = [S21, S22, S23, S24],
    Row3 = [S31, S32, S33, S34],
    Row4 = [S41, S42, S43, S44],

    Col1 = [S11, S21, S31, S41],
    Col2 = [S12, S22, S32, S42],
    Col3 = [S13, S23, S33, S43],
    Col4 = [S14, S24, S34, S44],

    Square1 = [S11, S12, S21, S22],
    Square2 = [S13, S14, S23, S24],
    Square3 = [S31, S32, S41, S42],
    Square4 = [S33, S34, S43, S44],

    valid([Row1, Row2, Row3, Row4,
           Col1, Col2, Col3, Col4,
           Square1, Square2, Square3, Square4]).

Well, copying my answer from SO.

Think about the number of outputs of subset/4, if every cell of Solution is unbound in the beginning. There are 44*4 = 4,294,967,296 outputs. Now you might know why your code is so slow.

You should call valid/1 inside of subset/2. Test if the puzzle is still valid after inserting one number:

subset(_,[],_).
subset(RCS,[H|T],List):-
    member(H,List),
    valid(RCS),
    subset(RCS,T,List).

% Like \+member(H,T) but unbound cells are ignored
nonmember(_, []).
nonmember(H1, [H2|T]) :-
    (var(H2); H1 \= H2),
    nonmember(H1, T).

different([]).
different([H|T]):-
    (var(H); nonmember(H,T)),
    different(T),!.

valid([]).
valid([Head|Tail]) :-
    different(Head),
    valid(Tail),!.

sudoku(Puzzle) :-
    Puzzle = [S11, S12, S13, S14,
              S21, S22, S23, S24,
              S31, S32, S33, S34,
              S41, S42, S43, S44],

    Row1 = [S11, S12, S13, S14],
    Row2 = [S21, S22, S23, S24],
    Row3 = [S31, S32, S33, S34],
    Row4 = [S41, S42, S43, S44],

    Col1 = [S11, S21, S31, S41],
    Col2 = [S12, S22, S32, S42],
    Col3 = [S13, S23, S33, S43],
    Col4 = [S14, S24, S34, S44],

    Square1 = [S11, S12, S21, S22],
    Square2 = [S13, S14, S23, S24],
    Square3 = [S31, S32, S41, S42],
    Square4 = [S33, S34, S43, S44],

    RCS = [Row1, Row2, Row3, Row4,
           Col1, Col2, Col3, Col4,
           Square1, Square2, Square3, Square4],

    subset(RCS, Puzzle, [1,2,3,4]).

Next optimization would be not to test all rows, columns and squares after every insert, but only the affected ones (3 instead of 12):

subset([], _).
subset([(P,RCS) | T], List):-
    member(P, List),
    valid(RCS),
    subset(T, List).

% Like \+member(H,T) but unbound cells are ignored
nonmember(_, []).
nonmember(H1, [H2|T]) :-
    (var(H2); H1 \= H2),
    nonmember(H1, T).

different([]).
different([H|T]):-
    (var(H); nonmember(H,T)),
    different(T),!.

valid([]).
valid([Head|Tail]) :-
    different(Head),
    valid(Tail),!.

sudoku(Puzzle) :-
    Puzzle = [S11, S12, S13, S14,
              S21, S22, S23, S24,
              S31, S32, S33, S34,
              S41, S42, S43, S44],

    Row1 = [S11, S12, S13, S14],
    Row2 = [S21, S22, S23, S24],
    Row3 = [S31, S32, S33, S34],
    Row4 = [S41, S42, S43, S44],

    Col1 = [S11, S21, S31, S41],
    Col2 = [S12, S22, S32, S42],
    Col3 = [S13, S23, S33, S43],
    Col4 = [S14, S24, S34, S44],

    Square1 = [S11, S12, S21, S22],
    Square2 = [S13, S14, S23, S24],
    Square3 = [S31, S32, S41, S42],
    Square4 = [S33, S34, S43, S44],

    RCS11 = (S11, [Row1, Col1, Square1]),
    RCS12 = (S12, [Row1, Col2, Square1]),
    RCS13 = (S13, [Row1, Col3, Square2]),
    RCS14 = (S14, [Row1, Col4, Square2]),
    RCS21 = (S21, [Row2, Col1, Square1]),
    RCS22 = (S22, [Row2, Col2, Square1]),
    RCS23 = (S23, [Row2, Col3, Square2]),
    RCS24 = (S24, [Row2, Col4, Square2]),
    RCS31 = (S31, [Row3, Col1, Square3]),
    RCS32 = (S32, [Row3, Col2, Square3]),
    RCS33 = (S33, [Row3, Col3, Square4]),
    RCS34 = (S34, [Row3, Col4, Square4]),
    RCS41 = (S41, [Row4, Col1, Square3]),
    RCS42 = (S42, [Row4, Col2, Square3]),
    RCS43 = (S43, [Row4, Col3, Square4]),
    RCS44 = (S44, [Row4, Col4, Square4]),

    RCS = [RCS11, RCS12, RCS13, RCS14,
           RCS21, RCS22, RCS23, RCS24,
           RCS31, RCS32, RCS33, RCS34,
           RCS41, RCS42, RCS43, RCS44],

    subset(RCS, [1,2,3,4]).

Now you should be able to increase the puzzle size to 9×9 and still have a reasonably fast solver.


Ogg audio format conversion classes This is my command interface:

public interface IConverter {
    void convert();
}

This is my Receiver class:

public class Ogg extends Audio{

    private File src;
    private File trgt;

    public static final String CODEC = "libvorbis";
    public static final String FORMAT = "ogg";

    public Ogg(File src, File trgt){
        this.src = src;
        this.trgt = trgt;
    }

    public void convertToOgg(){
        audioAttr.setCodec(CODEC);
        encoAttrs.setFormat(FORMAT);
        encoAttrs.setAudioAttributes(audioAttr);
        try {
            encoder.encode(src, trgt, encoAttrs);
        } catch (IllegalArgumentException e) {
             e.printStackTrace();
        }
    }
}

And this is my Concrete Command:

package org.hitplay.audio.converters;

public class OggConverter implements IConverter {

    private Ogg ogg;

    public OggConverter(Ogg ogg){
        this.ogg = ogg;
    }
    @Override
    public void convert() {
        ogg.convertToOgg();
    }
}

This is my Invoker class:

public class AudioConverter {
    IConverter audio;

    public AudioConverter(IConverter audi){
        this.audio = audi;
    }
    public void setAudio(IConverter audio){
        this.audio = audio;
    }
    public void convert(){
        audio.convert();
    }
}

I have currently studied the command design pattern on this link and I was wondering if I have implemented the design pattern correctly. f I do not, please tell me why, and how else I can improve this code. Also, I have other classes besides Ogg and OggConverter; I also have Mp3 and Mp3Converter.

I do not see the reason of for class OggConverter as it is now.

Instead of that you could just add the interface IConverter to class Ogg. And remove all others classes as then only forward the call.

IConverter converter = OggConverter(Ogg(file1,file2));

If we add the interface to Ogg

IConverter converter = Ogg(file1,file2);

I also do not see the reason for class AudioConverter, beside that it can store the ref to IConverter but we can store this reference in other place.

In addition to improve are those CODEC and FORMAT string constant. Instead of string constatns you shuld provide enumerations.

public enum AudioCodec {
  LIBVORBIS;
}

public enum AudioFormat {
 OGG("ogg"),
 MP3("mp3");

private final String ext;

private AudioFormat(String ext) {
  this.ext = ext;
}

public String getExt() {
   return this.ext;
 }
}


Unroll for loop as macro to make array unmutable I wrote these lines of rust yesterday. It turns 27 bits from a 32 bit integer into a 3x9 byte array, but my gut feeling is this should be done without having a mutable variable ans.

pub fn u32_to_preformated_vector(image:u32) ->[[u8;9];3] {
  let mut ans:[[u8; 9]; 3]= [[0,0,0,0,0,0,0,0,0],[0,0,0,0,0,0,0,0,0],[0,0,0,0,0,0,0,0,0]];
  let image = !image;
  for i in 0..27 {
     ans[i/9][i%9] = 1 & (image >> i) as u8;
  }
  ans
}

What would make the cleanest code here? A macro or some iterator magic?

Please also see also the code here: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=574d96b16d82c5816ebc35c5d88acda2

The name of the function u32_to_preformated_vector let me assume I get something else that what I'm actually get. So I think the naming could improve:

the function actually does not return a vector, but an array;
the function does invert the input bits. That's completely invisible in the name.
You might even indicate the bit order you handle the conversion in, as not everybody might expect to get the least significant bit/the least significant nonet first.

Keeping the mutable array, I came to this code:

pub fn u32_to_preformatted_array(image: u32) -> [[u8; 9]; 3] {
    let mut result = [[0u8; 9]; 3];
    let inverted = !image;
    for n in 0..3 {
        let nonet = inverted >> n * 9;
        for m in 0..9 {
            result[n][m] = 1 & (nonet >> m) as u8;
        }
    }
    result
}

I thought it would help if I bind the inverted image to a new name (inverted). That way in the for loop the reader sees that it is a different value to the one that was passed to the function as the argument.

I also converted the single for loop to two nested for loops. While nesting these loops in general might cause performance problems, it does no harm here, as the number of iterations in the inner code keep the same (27). That way I think the index handling is clearer than doing a divide and a remainder to calculate them. It might also make the bit order more obvious.

The initial value of the array can be initialized in a compacter way as all values in it are the same (0u8 in this case).

I don't think having a mutable array inside the function is a big deal. While I very much go for immutability in general, here the context within which mutation is possible is quite limited. As this is the place where the value gets built, there is no danger of it being used at a different place. From the point you return the value, it will be immutable by the declaration of your function.

I was also trying to get rid of this single mut by using iterators. It's quite easy as long as you change the return type of the function from arrays to vectors:

pub fn u32_to_preformatted_vec(image: u32) -> Vec<Vec<u8>> {
    let inverted = !image;
    (0..3)
        .map(|i| (inverted >> 9 * i) & 0b111_111_111) // `&` not strictly needed
        .map(|nonet| (0..9).map(|i| 1 & (nonet >> i) as u8).collect())
        .collect()
}

In general I have the impression working in a map/reduce style works a lot better with Vec than on arrays. You can also convert the Vec to a slice, but the conversion to an array is harder and often is done using functions like .clone_from_slice() which require a previously allocated array that is mutable again. I also tried to get a slice for the Vec and then use the TryFrom trait to convert it to an array. While I got this working for single dimensional array, I didn't get it to work for the multidimensional array [[u8; 9]; 3].

There is also rarely a need to go from a Vec to an array when designing the solution. Vec is just a little bit of code around an array (that's why to can get a slice from it so easily). So you don't really save anything when using an array.

Thinking about the whole problem as such, I wonder what's the reason for this conversion. You're blowing a 4 byte value up to using 27 bytes of memory. That way you increase the memory usage of your program, and I don't know what you get for it. I don't think that it will actually improve performance because you loose locality when accessing the data. The caching in your computer will not work as good as by using the original 4 byte value.

If the goal you try to get from the conversion is improved readability, I think you might try to define helper functions to get (or set) the individual bits. The calculations are actually quite easy, are performed in the CPU registers and can be inlined by the compiler. That should be quite fast … faster than converting the data to this two dimensional array which requires a lot of access to the computer's memory because it cannot be done in registers.


Formatting MVC 5 Razor link as a string MVC 5 doesn't seem to like JavaScript that much.
I am creating link in JavaScript to a success method in my Ajax call.

Razor:

@Html.ActionLink("Amount", "Action", "Controller", new { id1 = "a", id2 ="b"}, new { target = "_blank"})

JavaScript:

$('#element').html('' + data[1] + '');

This link works, but, I am not sure it's the best solution. Tried with a onclick method on the element instead, and with a separate method for handling the link, but still doesn't really get around this replacing.

And if it has more parameters should I just be chaining them?

.replace("__id2__", id2).replace("__id3__", id3).replace("__id4__", id4) etc

I can't really comment on your structure in Razor, as I am unfamiliar with that framework, however, your replacing structure could be improved.

By using a function, alongside a dictionary, you can simplify this greatly.

function massReplace(str, dict){
    for (var i in dict){
        str = str.replace(i, dict[i]);
    }
    return str;
}

Whilst also converting the "__id3__", id3 structure into:

var dict = {
    "__id2__": id2,
    "__id3__": id3,
    "__id4__": id4
};

And then simply:

$('#element').html('' + data[1] + '');


Nested template variable access in Meteor I'm a brand new JavaScript guy (currently playing with Meteor) and would love some critique of the following:

The goal:  I've got two collections

Buckets
Widgets.

I'm displaying a select for each bucket, populated with an option for each widget.

The goal is simply to mark the appropriate option as selected if the bucket currently contains that widget.

I wrestled with this a bit and came up with what feels hack-ey

-- I'm explicitly passing the bucket's contents as a Handlebars variable.  I feel like there's a better way, but not sure how to get 'er done.

Is there a way to get access to the parent template instance's context from a nested template instance?  #each widget is being called from within #each bucket but this only gives me access to the child context.

(This works, I'm just trying to learn here).

HTML:

<template name="choices">
  {{#each bucket}}
    <label for='{{bucket_name}}'>{{bucket_description}}</label>
    <select id='{{bucket_name}}'>
      {{#each widget}}
        <option value="{{widget_name}}" {{selected ../bucket_contains}}>{{widget_description}}</option>
      {{/each}}
    </select>
  {{/each}}
</template>

JS:

if (Meteor.isClient) {
  Template.choices.bucket = function () {
    return Buckets.find({});
  };

  Template.choices.widget = function () {
    return Widgets.find({});
  };

  Template.choices.selected = function (parent) {
    return (this.widget_name === parent) ? 'selected' : '';
  };

  Template.choices.events({
    'change select': function (event) {
      Buckets.update({bucket_name: event.target.id}, {$set: {bucket_contains: event.target.value}});
    }
  });
}

I've been looking at your code for a few weeks now, the problem is both that there is not a lot to review and there's nothing wrong with it.

Do consider using lowerCamelCasing which is the standard in JS, so bucket_name -> bucketName.

The code seems elegant enough to me.


Cleaning up repeated calls to NSMakeRange() In Objective-C, many of the NSString or NSMutableString methods for comparing or manipulating strings require a range argument--that is, an argument of data type NSRange (a struct containing a starting position and a length).  This applies outside of NSString (NSArray for example), but NSString is where I see it most frequently.

There does exist a convenience function for creating an NSRange struct:

NSRange range = NSMakeRange(position, length);

But the problem for me happens when I'm manipulating a string.  I'll sometimes write a method in which I will make several different manipulations on the string within the same method.  And the range I need to cover will be the entire length of the string.  It's not acceptable to simply declare an NSRange variable, set it, and use this throughout, because if the length of the string changes, the previously set range will be incorrect.

For example, suppose we're working with strings representing file paths.  We want to manipulate the string in a few ways to make sure the path looks how we want it to look:

- (NSString *)cleanPath:(NSString *)originalPath inRoot:(NSString *)rootPath {
    NSString *returnPath = [originalPath stringByReplacingOccurrencesOfString:@" " withString:@"" options:NSCaseInsensitiveCompare:NSMakeRange(0,[originalPath length])];

    while ([returnPath rangeOfString:@"./"].location != NSNotFound) {
        returnPath = [returnPath stringByReplacingOccurrencesOfString:@"./" withString:@"/" options:NSCaseInsensitiveCompare:NSMakeRange(0,[returnPath length])];
    }

    if ([returnPath rangeOfString:rootPath options:NSCaseInsensitiveCompare range:NSMakeRange(0,[returnPath length])] != NSNotFound) {
        return returnPath;
    } else {
        return rootPath;
    }
}

As you can see, we have three calls to NSMakeRange just to make a range that encompasses the length of the string we're working on.  We want to look at the whole range.  Giving a range too short will mean we're not looking at every character, and giving a range too long will result in an out of bounds exception.  We also can't know what the range will be.

We know that the first range will be the longest range, because we're only removing characters.

Also, even though the call only shows up in code once in the while loop, it could be called any number of times in actuality, so we can't even preset a range before the loop and use that number every time.

The other annoying thing is that NSMakeRange(0,[myString length]) doesn't offer a ton of autocomplete help.

I also don't want to simply #define a macro that could accidentally have unintended consequences outside of this method.

What are my options to clean this up?

This is tagged C because even though the code here is Objective-C, NSRange is a C-style struct and NSMakeRange is a C-style function.  Any C techniques for cleaning up something like this will definitely apply here.

I see a couple of ways to clean this up.

First of all, you mention not wanting to use #define, but let's not forget about #undef.  You could define a macro as the first line of this method and undefine it as the last line of the method.

For example:

- (NSString *)cleanPath:(NSString *)originalPath inRoot:(NSString *)rootPath {
   #define _STRING_RANGE(string) (NSMakeRange(0,[string length]))
   // code
   #undef _STRING_RANGE
}

Now, this macro definition exists only within this method. You don't necessarily save a whole lot of characters, but autocomplete will be a lot nicer, and you don't have to explicitly tell the macro to start at 0 and go to the end...you've predefined that in the macro.

You can also just write your own C-Style function.  That's all NSMakeRange is after all.

NSRange stringRange(NSString *string) {
    return NSMakeRange(0,[string length]);
}

And while defining a function is fine and dandy, now it can be seen by at least everything in the file.  This is Objective-C, so we can use blocks.  We can create a block within the method, and now the block, much like the #define macro that we #undef exists only within the method:

- (NSString *)cleanPath:(NSString *)originalPath inRoot:(NSString *)rootPath {
    NSRange (^stringRange)(NSString *) = (^NSString *string) {
        return NSMakeRange(0,[string length]);
    }
    // code...
}

Here we've define a block called stringRange, that returns a NSRange and takes a single argument of type NSString *.  This is exactly the same as the function we wrote in option two, and it's even called in the exact same way too.

stringRange(myString)

There a couple of important differences between a regular C-style function and a block though.  Scope and memory.

With this block example, the block's scope is limited only to this method.  It also exists in memory on the stack and only until this method returns (or until the end of an @autoreleasepool it was created in, whichever comes first).


Calculating pi with Monte Carlo using OpenMP I'm a C noob and I'm learning about concurrency using C. I came across an exercise in a book asking me to find the approximate value of Pi using the Monte Carlo technique with OpenMP. I came up with the following:
#include <stdio.h>
#include <stdlib.h>
#include <pthread.h>
#include <math.h>
#include <omp.h>

int in_circle = 0;
int total = 10000;
int thread_numb = 4;

void calculate_in_circle_random_count() {
  srand((unsigned int)time(NULL));
  for (int i = 0; i < total / thread_numb; i++) {
    float x = (float)rand()/(float)(RAND_MAX);
    float y = (float)rand()/(float)(RAND_MAX);

    float val = (x * x) + (y * y);

    if (val < 1.0) {
      #pragma omp critical
      {
        in_circle++;
      }
    }
  }
}

int main(int argc, char const *argv[]) {
  #pragma omp parallel num_threads(thread_numb)
  {
    printf("Hello from process: %d\n", omp_get_thread_num());
    calculate_in_circle_random_count();
  }

  float pi_approx = 4 * (float)in_circle / (float)total;

  printf("In circle: %d\n", in_circle);
  printf("Total: %d\n", total);
  printf("Pi approximation: %.6f\n", pi_approx);

  return 0;
}

I have the sample count of 10000 to calculate approximation and I want to dedicate 4 threads to be parallel for the calculation. So since I want 4 threads, I make sure the for loop is run until total / 4 for each thread. And whenever (val < 1.0) is true I increase the in_circle variable in a critical section.
Does this approach make sense? If not, how could it be improved?

This code has a serious problem due to (at least the typical implementation of) rand(). rand() normally has a (hidden) seed value, and each call to rand() modifies the seed. At least in most implementations, that means rand always forces serialization.
That means, calling rand() (a couple of times) in your inner loop will prevent code from scaling well at all. In fact, in most cases (as Toby Speight showed) a multi-threaded version will run substantially slower than a single-threaded version.
To fix this, you pretty much need to use some other random number generator. Your implementation may provide one (e.g., erand48 is fairly common). If you really need your code to be portable, you could write your own, something on this order:
#include <time.h>

typedef unsigned long long rand_state;

// multiplier/modulus taken from Knuth Volume 2, Table 1
static const int multiplier = 314159269;
static const int addend = 1;
static const int modulus = 0xffffffff;

// note that this works differently from srand, returning a seed rather than setting
// a hidden seed.
rand_state omp_srand() {
    rand_state state = time(NULL);
    state ^= (unsigned long long)omp_get_thread_num() << 32;
    return state;
}

int omp_rand(rand_state *state) {
    *state = *state * multiplier + addend;
    return *state & modulus;
}

Note that since it's entirely likely that all the threads get started in the same second, this combines the current time with the thread ID to seed each thread's generator. Although it's not 100% guaranteed, this gives a pretty high likelihood that each thread's generator will start with a unique seed. On the other hand, it also means that we have to start the thread, and then seed the generator inside that thread.
To use this, our code would be something on this general order:
double calculate_in_circle_random_count(void) {

    static const unsigned total = 1000000000;
    unsigned in_circle = 0;

#pragma omp parallel reduction(+:in_circle)
    {
        // do per-thread initialization
        rand_state seed = omp_srand();
        int count = total / omp_get_num_threads();
        int i;

        // then do this thread's portion of the computation:
        for (i = 0;  i < count;  ++i) {

            double x = (double)omp_rand(&seed) / OMP_RAND_MAX;
            double y = (double)omp_rand(&seed) / OMP_RAND_MAX;

            double val = x * x + y * y;
            in_circle +=  val < 1.0;
        }
    }
    return 4.0 * in_circle / total;
}

int main(int argc, char const *argv[])
{
    int num_threads = 1;

    if (argc > 1)
        num_threads = atoi(argv[1]);

    omp_set_num_threads(num_threads);

    float pi_approx = calculate_in_circle_random_count();

    printf("Pi approximation: %.6f\n", pi_approx);

    return 0;
}

With this modification, the code is at least capable of scaling. To get it to scale very well, you'd need to change your total to a rather larger number though--with it as small as you've specified, it takes longer to start up multiple threads than it saves in calculation. But at least this code can scale well, so if we make total quite a bit larger, it really will run faster. For timing, I added a few more zeros to total, and rewrote main a bit, to create a loop to use 1, 2, 3, and 4 threads and print out the time each iteration. On my machine, this produced the following:
With 1 threads: Pi approximation: 3.140259, time: 3,652,683 microseconds
With 2 threads: Pi approximation: 3.139389, time: 1,892,415 microseconds
With 3 threads: Pi approximation: 3.138885, time: 1,268,917 microseconds
With 4 threads: Pi approximation: 3.138306, time:   935,579 microseconds

So with this, 4 threads is at least close to 4 times as fast as one thread. I'm pretty sure if you use rand inside the loop, you'll never get it to scale well at all.
For anybody who cares, the version of main that produces that output is written in C++, and looks like this:
int main(int argc, char const* argv[]) {
    using namespace std::chrono;

    std::cout.imbue(std::locale(""));

    int processors = omp_get_num_procs();

    for (int num_threads = 1; num_threads <= processors; num_threads++) {
        std::cout << "With " << std::setw(2) << num_threads << " threads: ";
        omp_set_num_threads(num_threads);

        auto start = high_resolution_clock::now();
        float pi_approx = calculate_in_circle_random_count();
        auto stop = high_resolution_clock::now();

        printf(" %.6f, time: ", pi_approx);
        std::cout << std::setw(9) << duration_cast<microseconds>(stop - start).count() << " microseconds\n";
    }
    return 0;
}

This remains pretty much the same, but with some timing code hacked in. In particular, it's not an attempt at rewriting the code in C++ in general. If I were doing that, I'd probably do a number of things rather differently (starting with the fact that the C++ <random> header already provides a clean way to handle per-thread random number generation, so I'd use that instead of rolling my own).


Perl CGI script to respond to erroneous Base64 image requests I often get lines like this in my Apache error log:

  File does not exist: /path/to/www/data:image/gif;base64,R0lGODlhBgAGAIAOAP/yH5BAEACAEALAAAAAAGGAYAAAIJhB0Xi5vOoFwFADs=

Obviously, this is due to somebody using an outdated browser (or a robot) which doesn't handle inline images (Data URI/base64 encoded).

I am thinking it must be easy to redirect requests starting with "data:image/gif;base64," to a short perl CGI script which returns the requested value as a binary image, with the correct content header (both GIFs and PNGs are used), and without having to read any image files.

I am thinking something along the lines of the following CGI (after a rewrite places the request URL in the query string):

#!/usr/bin/perl
use strict;
use warnings;
use MIME::Base64 ();
$ENV{QUERY_STRING} =~ m{^data:(\w+/\w+);base64,(.+)$} or die "bad input";
print "Content-Type: $1\n\n";
print MIME::Base64::decode ($2);

Would that work and would there be any downsides to this approach (apart from accommodating people with last-century browsers)?

The only real problem I see is that if the browser is really "last century" it probably won't support a long enough query string to contain the full image. So your returned image might be incomplete. Another thing is that this would waste a lot of bandwidth.

I see two options

To suppress those error messages you could just handle those URLs like you and make a header redirect (to use the browser cache!) to an error/empty image.
Use conditional comments to load inline images on IE < 8 with a normal img:src attribute. Since your culprit is most likely an Internet Explorer (people who install alternate browsers usually keep them up to date)

I would use the first actually, as it does not involve changing code.

To test whether your solution works, start Internet Explorer (9), open the Web Development tools, and dial it back to IE 6 or 7 (best try both)