Send multiple individualised emails via a limited number of SMTP clients This is my first foray into multi-threading, and I'm keen to know if I have fallen into any traps or have room for optimisation/improvement.

Example usage:

var success = new ConcurrentBag<CourseParticipant>();
var fail = new ConcurrentBag<CourseParticipant>();
using (var parallelEmails = new ParallelSmtpEmails())
{
     foreach (var cp in courseParticipants)
     {
        var mail = new MailMessage{
            From = new MailAddress("from@example.com"),
            To = cp.email,
            Subject = "come and attend course",
            BodyFormat = MailFormat.Html
        }
        string token = await GetRegistrationTokenAsync(cp);
        mail.Body = $"
let me come
"
        parallelEmails.Send(mail, s=>
        {
            if (s == null)
            {
                success.Add(cp);
            }
            else
            {
                fail.Add(cp);
            }
            mail.Dispose();
        });
     }
     await parallelEmails.SendingComplete();
}

In the code below, I am particularly wondering about the RunWorker method, the do while loop, and if workList.TryGet. If there is nothing in the worklist, this thread will remain looping until CompletedAdding is called or more emails are added. Is this looping an acceptable way to wait the thread?

Being an amateur, I am also grateful for any other code improvements or style points.

public class ParallelSmtpEmails : IDisposable
{
    BlockingCollection<MailTask> _workList;
    List<Task> _tasks;
    int _maxClientCount;

    public ParallelSmtpEmails(int maxSmtpClients = 5)
    {
         //removing the boundedCapacity := maxSmtpClients from BlockingCollection instantiator - the collection can be filled up without blocking
        _workList = new BlockingCollection<MailTask>(new ConcurrentQueue<MailTask>());
        _tasks = new List<Task>(maxSmtpClients);
        _maxClientCount = maxSmtpClients;
    }

    public void CompletedAdding()
    {
        _workList.CompleteAdding();
    }

    public void Send(MailMessage message)
    {
        Send(message, null);
    }

    public void Send(MailMessage message, Action<SmtpException> onComplete)
    {
        _workList.Add(new MailTask() { Message = message, OnComplete = onComplete });
        if (_tasks.Count < _maxClientCount && _workList.Count > 0)
        {
            _tasks.Add(Task.Factory.StartNew(RunWorker));
        }
    }

    public Task SendingComplete()
    {
        CompletedAdding();
        return Task.WhenAll(_tasks);
    }

    public void RunWorker()
    {
        using (var client = new SmtpClient())
        {
            do
            {
                MailTask m;
                if (_workList.TryTake(out m))
                {
                    try
                    {
                        client.Send(m.Message);
                        m.OnComplete?.Invoke(null);
                    }
                    catch (SmtpException ex)
                    {
                        m.OnComplete?.Invoke(ex);
                    }
                }
            } while (!_workList.IsCompleted);
        }
    }

    #region IDisposable
    bool _disposed;
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~ParallelSmtpEmails()
    {
        Dispose(false);
    }

    void Dispose(bool disposing)
    {
        if (_disposed)
            return;

        if (disposing)
        {
            // free other managed objects that implement
            // IDisposable only
            _workList.Dispose();
        }

        // release any unmanaged objects
        // set the object references to null

        _disposed = true;
    }
    #endregion //IDisposable

    private class MailTask
    {
        public MailMessage Message { get; set; }
        public Action<SmtpException> OnComplete { get; set; }
    }
}

Don't use SmptClient.Send(), use SmptClient.SendMailAsync() instead. It returns a Task..

Actually, I don't think you need this class at all.

(I'm typing in a notepad, so this is untested and just to illustrate the concept.)

var mailTasks = new List<Task>();
var client = new SmtpClient();

foreach(var participant in courseParticipants)
{
    var mail = new MailMessage
    {
        From = new MailAddress("from@example.com"),
        To = participant.email,
        Subject = "come and attend course",
        BodyFormat = MailFormat.Html
    }

    string token = await GetRegistrationTokenAsync(cp);
    mail.Body = $"
let me come
";

    mailTasks.Add(client.SendMailAsync(mail));
}

try
{
    await Task.WhenAll(mailTasks);
}
catch(AggregateException ex)
{
    //handle failures
}

This code is much simpler to understand, and therefore maintain.

One last thing before I go: Are you absolutely certain that GetRegistrationTokenAsync() will always return a url safe string?

string token = await GetRegistrationTokenAsync(cp);
mail.Body = $"
let me come
";

You should probably be Url encoding the token.


Change case of string, replace hyphens with spaces and capitalize first letter I'm trying to transform the following string "joe-smith" to "Joe Smith". I have the following code which seems to work okay, but is there a faster, better way?

var prettify = function(str){
   var temp = str.split('-'), i, pretty;

   for(i = 0; i < temp.length; i++){
     temp[i] = temp[i].charAt(0).toUpperCase() + temp[i].slice(1);
   }

   pretty = temp.join(' ');

   return pretty;
 };

 var prettyString = prettify('joe-smith');
// outputs "Joe Smith"

Some things:

That pretty variable is pretty useless. Omit it.
temp is quite meaningless. Use a descriptive name like words or parts.
You could use the Array map method instead of that for loop. This will likely be slower, but more elegant:

function prettify(str) {
    return str.split('-').map(function capitalize(part) {
        return part.charAt(0).toUpperCase() + part.slice(1);
    }).join(' ');
}

regular expressions and the String replace method could further shorten it, and possibly improve performance:

function prettify(str) {
    return str.replace(/(-|^)([^-]?)/g, function(_, prep, letter) {
        return (prep && ' ') + letter.toUpperCase();
    });
}

Decide yourself whether this is too cryptic or not.


PHP OOP example using animal classes I've been reading up on OOP concepts such as: polymorphism, inheritance and encapsulation and would like to know if I am even close to using these concepts correctly.

interface Canine
{
    public function bark();
    public function howl();
}

interface Feline
{
    public function chase();
}

interface Mammal
{
    const GENDER_MALE = 'male';
    const GENDER_FEMALE = 'female';

    public function isMale();
    public function isFemale();
}

abstract class Animal
{
    public function breath()
    {
        return 'I breath oxygen found in the air using my nose.';
    }
}

class Dog extends Animal implements Canine, Mammal
{
    private $name;
    private $gender;

    public function __construct($name, $gender)
    {
        $this->setName($name);
        $this->setGender($gender);
    }

    private function setName($name)
    {
        $this->name = (string) $name;
    }

    private function setGender($gender)
    {
        if ($gender === self::GENDER_MALE || $gender === self::GENDER_FEMALE) {
            $this->gender = $gender;
        } else {
            throw new InvalidArgumentException('Mammals can only be male or female');
        }
    }

    public function getGender()
    {
        return $this->gender();
    }

    public function eat()
    {
        return 'I eat meat.';
    }

    public function bark()
    {
        return 'Woof, woof!';
    }

    public function howl()
    {
        return 'AHROOOOOOO WOO WOOO!';
    }

    public function isMale()
    {
        return $this->gender === self::GENDER_MALE;
    }

    public function isFemale()
    {
        return $this->gender === self::GENDER_FEMALE;
    }
}

class Lion extends Animal implements Mammal, Feline
{
    private $gender = self::GENDER_MALE;

    public function chase()
    {
        return 'I am chasing a Gazelle!';
    }

    public function isMale()
    {
        return $this->gender === self::GENDER_MALE;
    }

    public function isFemale()
    {
        return $this->gender === self::GENDER_FEMALE;
    }

    public function eat()
    {
        return 'I am now eating the gazelle I just caught.';
    }
}

class Fish extends Animal
{
    final public function breath()
    {
        return 'I breath oxygen found in the water using my gills.';
    }
}

class Client
{
    public function __construct()
    {
        $dog = new Dog('Sparky', Mammal::GENDER_MALE);
        $cat = new Lion;
        $fish = new Fish;

        $this->setupFeline($cat);
        $this->setupCanine($dog);

        $this->setupAnimal($cat);
        $this->setupAnimal($dog);
        $this->setupAnimal($fish);
    }

    public function setupFeline(Feline $cat)
    {
        echo 'Feline: ' . $this->iAmA($cat), ' and ',$cat->chase(), '&nbsp;', $cat->eat() . '
';
    }

    public function setupCanine(Canine $dog)
    {
        echo 'Canine: ' . $this->iAmA($dog), '&nbsp;', $dog->howl() . '
';
    }

    public function setupAnimal(Animal $animal)
    {
        echo $this->iAmA($animal) . ' and ' . $animal->breath() . '
';
    }

    private function iAmA(Animal $animal)
    {
        return 'I am a ' . get_class($animal);
    }
}

$client = new Client;

I agree with Gerard a little, you have made (in my eyes) a common error with interfaces where you've tried to make them specific to a certain type rather than what they actually do, I see this a lot in the wild (excuse the pun). This isn't to say that interfaces based on type are bad, they have their place but in general an abstract would be better suited for types as types will have unique properties that you could not declare in an interface (nor would you want to).

As an example I'll elaborate on the "Dog is also capable of chasing" example. Declaring a Dog as Feline makes no sense when trying to describe what a Dog is, instead, make it capable of doing something.

/**
 * Any animal that chases after things should implement
 * this interface.
 */
interface Chaser {
    /**
     * GET THE RABBIT!
     * @return void
     */
    public function chase();
}

On the flipside, you could easily have an interface such as:

/**
 * Any animal that is chased by another implements this
 */
interface Chasable {
    /**
     * RUN RABBIT!
     * @return void
     */
    public function beingChased();
}

So you now have interfaces that describe what they do. It now makes sense that both a Dog and a Lion can implement Chaser. Not only this, any animal class you create are capable of using it without semantic issues.

Your Canine interface suffers from the same problem (to a lesser degree). Having a canine interface is probably not a bad thing, declaring that canines only bark or howl is true. Is it true to say that no other animal species can bark or howl? what happens when this cat turns up on your doorstep? (stupid example but I found it amusing)

Cat videos aside, we now have a strange edge case where a cat is displaying canine behaviour, but it is not a canine. Enter the Barker interface (that I find both funny and embarrassing).

/**
 * Any animal that barks can extend this interface
 */
interface Barker {
    /**
     * WOOOF!!
     * @return void
     */
    public function bark();
}

Going into the nuances of having a single Cat that can bark out of a set of possibly hundreds of Cat instances is beyond what you're trying to learn but I'm hoping you get the gist of where I'm going with this, behaviour as interfaces generally work better than types and give you a lot more flexibility when actually determining what your classes do.

I'm making general statements a lot here, but you will see that your interfaces become very specific to actions. As such they are smaller and make more sense. I have found it much easier to have a class that implements 5 smaller interfaces than 1 or 2 monolithic interfaces due to the increased flexibility available and it keeps things DRY.

class Canine extends Mammal implements Chaser, Barker, Howler
{
    // I am a Canine. I am a mammal. I do things that all mammals do
    // (that also includes being an animal).
    // I can chase, bark and howl. I know this before I've written
    // any methods because my parent and interfaces say so.
}

Some tips for you moving forward:

Getters and setters are good, but can clog up your class with menial methods that achieve very little when you are not modifying data passed in or out.
Interfaces work best when they describe what something should do.
Don't rely on just interfaces to achieve what you need. I'd recommend getting stuck into common OOP design patterns sooner rather than later.

Example of the chaser and chasable interfaces in action:

First lets make our interfaces a little more rounded

/**
 * Any animal that chases after things should implement
 * this interface.
 */
interface Chaser {
    /**
     * GET THE CHASABLE ANIMAL!
     * @param Chasable
     * @return string
     */
    public function chase(Chasable $chasable);
}

/**
 * Any animal that chases after things should implement
 * this interface.
 */
interface Chasable {
    /**
     * RUN FROM THE CHASER!
     * @param Chaser
     * @return string
     */
    public function beingChased(Chaser $chaser);
}

Now let's create the chaser and the chasable animals.

/**
 * A chaser animal
 */
class Dog implements Chaser {
    /**
     * GET THE CHASABLE ANIMAL!
     * @param Chasable
     * @return string
     */
    public function chase(Chasable $chasable)
    {
        return "I'm chasing a {get_class($chasable)}!"
    }
}

class Rabbit implements Chasable {
    /**
     * RUN FROM THE CHASER!
     * @param Chaser
     * @return string
     */
    public function beingChased(Chaser $chaser)
    {
        return "Help! I'm being chased by a {get_class($chaser)}"
    }
}

Start the chase!

$dog = new Dog();
$rabbit = new Rabbit();

//We know the dog is a chaser and the rabbit is a chasable
//So we know that this will work
echo $dog->chase($rabbit);

//We know the rabbit is a chasable and the dog is a chaser
//So we know that this will work
echo $rabbit->beingChased($dog);


Repeated sequential subsetting of data.table I have a long data.table, payoff_mat.  I would like to select the rows containing the maximum value of column X by group Y. There are several posts on this, and I think I am well optimized in this regard.

The twist is that I would like to repeat this process several times, for which I think I could do better.  This is the hot-spot of my script.

This is how it works:

Subset the rows containing the maximum value of column 3 by group (permutation of column 4-5).
Subset again by the maximum of column 2 by group (permutation of column 4).
Subset again by the maximum of column 1.

library(data.table)
library(iterpc)

set.seed(1)
I<-iterpc(table(rnorm(5)),3, ordered=TRUE,replace=FALSE)
support_n <-getnext(I,d=180) # not real size. Larger in practice.
payoff_n <-matrix(rnorm(180), ncol = 3) # not real size. Larger in practice.
payoff_mat<-cbind(payoff_n,support_n)
payoff_mat<-data.table(payoff_mat)

#My current solution:  eval(parse(text = ....)) is long but seems to be the standard best syntax for data.table in these cases.

cn<-colnames(payoff_mat) #need column names, data.table does not like using column number.
step_1<-payoff_mat[payoff_mat[ ,.I[ which.max(eval(parse(text = cn[3]))) ] , by = eval(cn[4:5]) ][,V1]] #
step_2<-step_1[step_1[ ,.I[ which.max(eval(parse(text = cn[2]))) ] , by = eval(cn[4:4]) ][,V1]]
step_3<-step_2[ ,.SD[ which.max(eval(parse(text = cn[1]))) ] ]

My intuition suggests I lose a lot of time writing to memory by saving each step, but I am happy with any improvement.

I have tried calling it as a single command, but have not been successful  figuring out the correct technique for sequential [] calls.  Below is my (current) best attempt, but it is clear they are not equivalent:

payoff_mat [payoff_mat[ ,.I[ which.max(eval(parse(text = cn[3]))) ] , by = eval(cn[4:5]) ][,V1]]  [payoff_mat[ ,.I[ which.max(eval(parse(text = cn[2]))) ] , by = eval(cn[4:4]) ][,V1]]  [ ,.SD[ which.max(eval(parse(text = cn[1]))) ] ]

I am not a data.table expert so I can't tell if there is a faster approach, though https://stackoverflow.com/q/16573995/1201032 suggests you are already using something efficient at each step.

The changes I have to suggest would however make your code a lot easier to read and maintain. First, define a function that can be applied at each step:

max_subset <- function(dt, var, by)
  dt[dt[, .I[which.max(get(var))], by = by][,V1]]

Then, you can make your successive calls as follows:

step <- payoff_mat
step <- max_subset(step, "V3", c("V4", "V5"))
step <- max_subset(step, "V2", c("V4"))
step <- max_subset(step, "V1", c())

Or using the magrittr package:

library(magrittr)
payoff_mat %>% max_subset("V3", c("V4", "V5")) %>%
               max_subset("V2", c("V4")) %>%
               max_subset("V1", c()) -> step3

Note that assigning intermediate outputs to a variable in the first approach (step <- ...) should not carry any significant time (feel free to test). And your code will be a lot more readable than using nested calls: max_subset(max_subset(max_subset(...), ..., ...).


Switch case statement refactoring I have the following code with switch case statement:

public string IrregularCouponLabel
{
    get
    {
        switch ((ctrl.IrregularCouponFirst ? 10 : 0) + (ctrl.IrregularCouponLast ? 1 : 0))
        {
            case 11: return LocalString.Evaluate("label.Both");
            case 10: return LocalString.Evaluate("label.First");
            case 1: return LocalString.Evaluate("label.Last");
            default: return LocalString.Evaluate("label.None");
        }
    }
}

How should I refactor this code to be more clear and readable or how could I otherwise improve this code snippet?

UPDATE
According to valuable answers
I have updated the code in the following way,

    public string IrregularCouponLabel
    {
        get
        {
            return LocalString.Evaluate("label." + GetIrregularCouponValue());
        }
    }

    private string GetIrregularCouponValue()
    {
        bool first = ctrl.IrregularCouponFirst;
        bool last = ctrl.IrregularCouponLast;
        bool both = first && last;
        return both  ? "Both" :
               first ? "First" :
               last  ? "Last" :
                        "None";

    }

What do you think about this? Is it readable or not?

You can use an if statement and the conditional operator:

string label;
if (ctrl.IrregularCouponFirst) {
  label = ctrl.IrregularCouponLast ? "label.Both" : "label.First";
} else {
  label = ctrl.IrregularCouponLast ? "label.Last" : "label.None";
}
return LocalString.Evaluate(label);

You can also use only conditional operators. This way of chaining conditional operators checks take a bit of work to grasp the first time, but it's very compact:

return LocalString.Evaluate(
  ctrl.IrregularCouponFirst && ctrl.IrregularCouponLast ? "label.Both" :
  ctrl.IrregularCouponFirst ? "label.First" :
  ctrl.IrregularCouponLast ? "label.Last" :
  "label.None"
);


Find the kth largest element of an array Find the kth largest element in an array of integers. You may assume
  that:

  1 <= k <=size(array).

Any comments? Is there a more efficient solution?

public int findKthLargest(int[] nums, int k) {
    int p = 0;
    int numElements = nums.length;
    // create priority queue where all the elements of nums will be stored
    PriorityQueue<Integer> pq = new PriorityQueue<Integer>();

    // place all the elements of the array to this priority queue
    for (int n : nums){
        pq.add(n);
    }

    // extract the kth largest element
    while (numElements-k+1 > 0){
        p = pq.poll();
        k++;
    }

    return p;
}

Here is the median of medians algorithm which also takes \$O(n)\$ in the worst cases:

Divide the array into \$\frac{n}{5}\$ lists of 5 elements each.
Find the median in each sub array of 5 elements.
Recursively ﬁnd the median of all the medians, let's call it \$M\$.
Partition the array into two sub array 1st sub-array contains the elements larger than \$M\$. Let's say this sub-array is \$a1\$, while other sub-array contains the elements smaller than \$M\$. Let's call this sub-array \$a2\$.
If \$k \le |a1|\$, return selection(a1,k).
If \$k− 1 = |a1|\$, return \$M\$.
If \$k \gt |a1| + 1\$, return selection(a2,k −a1 − 1).

Let's see an interesting example of this where we have to find out the median of unsorted array in linear time. This may sounds NO at first look because median of array is middle element in sorted array if array is of odd length and middle of two middle elements if array is even length.

We can do it using quick/heap sort which takes \$O(nlogn)\$ time, but can we improve? Yes, have you heard about counting sort, which takes \$O(n)\$ time but extra \$O(Max−Min+1)\$ space, where Max and Min are the element of the array?

What if the array is ar[]={10000000,200000000}. Waste of space, right? But if you think about the median, which is nothing but the middle element of the array, where \$k = \frac{n}{2}\$ if array is odd length, can you see anything now? The median is nothing but \$\frac{n}{2}\$th smallest element in the array.

Great, so we got it. We can call median of medians algorithm up to \$k = \frac{n}{2}\$ and we can return ar[n/2] as the median.

Here's the pseudocode for the same:

//selects the median of medians in an array

static int selection(int a[], int s, int e, int k)

{

   // if the partition length is less than or equal to 5

   // we can sort and find the kth element of it

   // this way we can find the median of n/5 partitions

   if(e-s+1 <= 5)

   {

       Arrays.sort(a,s, e);

       return s+k-1;

   }

   /* if array is bigger we partition the array in sub-arrays of size 5
   no. of partitions = n/5 = (e+1)/5 iterate through each partition and
   recursively
   calculate the median of all of them and keep putting the medians in the
   starting of the array
   */

   for(int i=0; i<(e+1)/5; i++)

   {

      int left = 5*i;

      int right = left + 4;

      if(right > e) right = e;

      //left+(right-left)/2 median will be 3rd element e.g.ar[2] in zero index based array

      int median = selection(a,left, right, 2);

      swap(a[median], a[i]);

   }

  /*now we have array  a[0] = median of 1st 5 sized partition a[1] = median of
  2nd 5 sized partition and so on till n/5 to find out the median of these n/5
  medians we need to select the n/10th element of this set (i.e. middle of it)
 */

   return selection(a, 0, (e+1)/5, ((e+1)/10));

}

Call this function as:

selection(ar,0,ar.length-1,k) //k=n/2 which is median

Analysis of algorithm:

Dividing the array by 5 assures a worst-case split of 70-30 and at-least half of the medians are greater than the median-of-medians, hence at-least half of the \$\frac{n}{5}\$ blocks have at-least 3 elements and this gives a \$\frac{3n}{10}\$ split, which means the other partition is \$\frac{7n}{10}\$ in the worst case. That gives a time complexity of \$T(n) = T(\frac{n}{5}) + T(\frac{7n}{10}) + O(n)\$.

More reading:

Page on indstate.edu
The Median-of-Medians Algorithm | Hacker News


Mass and length calculator using Perl 6 custom operator I am studying chemistry in the university, and then I try to write all the things in the textbook with Perl6 or Perl, like balancing the chemical formula or other process!

Then I encountered the problem is on perl6 custom operator.
I feel I have been repeating my code and myself when i use the feature.
It is hard to read and write, and what is the way to deal with such problems?

#!/usr/bin/env perl6
use v6;
#basic SI(International System of Units) type

role MetricPrefix {
    method baseOn ( Str $base , Numeric $input ) {
        given $base {
            when 'pico' { return $input * 10**-12 }
            when 'namo' { return $input * 10**-9 }
            when 'micro' { return $input * 10**-6}
            when 'milli' { return $input * 10**-3 }
            when 'centi' { return $input * 10**-2 }
            when 'hecto' { return $input * 10**2 }
            when 'kilo' { return $input * 10**3 }
            when 'mega' { return $input * 10**6 }
            when 'giga' { return $input * 10**9 }
            when 'tera' { return $input * 10**12 }
            default { fail "you must input a metric prefix which allow pico to tera" }
        }
    }
}

class Mass does MetricPrefix {
    #basic Mass is g is different form si statda
    has $.g;

    submethod BUILD ( :$!g  ) {
    }

}

class Length does MetricPrefix {
    has $.Length ;

    submethod BUILD ( :$!Length  ) {
    }
}

multi postfix:<(kg)>( $input ) {
    return Mass.new( g => Mass.baseOn("kilo",$input) ) or fail "you Must input a number";
}

multi postfix:<(g)>( $input ) {
    return Mass.new( g => $input ) or fail "you Must input a number";
}

multi infix:<+>( Mass $inputOne , Mass $inputTwo ) is assoc<right> {
    return Mass.new( g => $inputOne.g + $inputTwo.g) or fail "error in there ";
}

multi infix:<->( Mass $inputOne , Mass $inputTwo ) is assoc<right> {
    return Mass.new( g => $inputOne.g - $inputTwo.g) or fail "error in there ";
}

multi infix:<*>( Mass $inputOne , Mass $inputTwo ) is assoc<right> is tighter( &infix:<+> ) is tighter( &infix:<-> ) is tighter( &infix:</>) {
    return Mass.new( g => $inputOne.g * $inputTwo.g) or fail "error in there ";
}

multi infix:</>( Mass $inputOne , Mass $inputTwo ) is assoc<right> is tighter( &infix:<+> ) is tighter( &infix:<-> )  {
    return Mass.new( g => $inputOne.g / $inputTwo.g) or fail "error in there ";
}

#the meterLeng
multi postfix:<(km)>( $input ) {
    return Length.new( Length => Length.baseOn("kilo",$input) ) or fail "you Must input a number";
}

multi postfix:<(m)>( $input ) {
    return Length.new( Length => $input ) or fail "you Must input a number";
}

multi infix:<+>( Length $inputOne , Length $inputTwo ) is assoc<right> {
    return Length.new( Length => $inputOne.Length + $inputTwo.Length) or fail "error in there ";
}

multi infix:<->( Length $inputOne , Length $inputTwo ) is assoc<right> {
    return Length.new( Length => $inputOne.Length - $inputTwo.Length) or fail "error in there ";
}

multi infix:<*>( Length $inputOne , Length $inputTwo ) is assoc<right> is tighter( &infix:<+> ) is tighter( &infix:<-> ) is tighter( &infix:</>) {
    return Length.new( Length => $inputOne.Length * $inputTwo.Length) or fail "error in there ";
}

multi infix:</>( Length $inputOne , Length $inputTwo ) is assoc<right> is tighter( &infix:<+> ) is tighter( &infix:<-> )  {
    return Length.new( Length => $inputOne.Length / $inputTwo.Length) or fail "error in there ";
}

#just a test
say 10(kg) + 1(g);
say 10(m) + 1(m);

This is an interesting use case of postfixes.  Here's how I would go about it.  Unfortunately, because postfixes can't take regexen, you need to specify each one individually, but it's not horrible, especially if you throw things into a module.

Let's first create a postfix operator for each value type:

multi postfix:<kg> (Numeric() $m) { Mass.new: $m, 'k' }
multi postfix:<g>  (Numeric() $m) { Mass.new: $m      }
multi postfix:<mg> (Numeric() $m) { Mass.new: $m, 'm' }

Basically, we're saying we want to attach g onto something.  We'd like it to be a number type, so we make have it only act on Numeric types, but if someone passes us a string, the () tells it to coerce it for us.  And then we make a Mass object from those the value and the prefix.

Now let's take a look at the Mass class.  All we really need to have in it is a weight and probably store its prefix so if we create a kg if we want to display it we can keep it in kg and not switch to mg or something.  We'll store things in grams, so on creation we use the prefix to shrink/expand the value.

class Mass {
  has $.grams;
  has $.prefix;
  method new  ($value, $prefix) {
    self.bless:
      :grams($value * si-prefix{$prefix}),
      :$prefix
  }
}

The si-prefix is just a map of values:

my constant \si-prefix := Map.new( 'k', 1000, '', 1, 'm', 0.001 );

By making it independent of the gram, we can use it later for a length / etc class.  For that reason, I'd define it outside of the class so that it can be used by others easily.  Believe it or not, it is possible to have an empty string as a key.  Kind of useful in our case.

At this point, we can start to use our values:

say 500g; # --> Weight.new(grams => 500, prefix => "")
say 2kg;  # --> Weight.new(grams => 2000, prefix => "kg")

Okay, not the most useful output, but at least we know everything is working.  Let's make a Str and gist method that makes more sense:

  method Str {
    $!grams / si-prefix{$!prefix}
    ~ $!prefix
    ~ 'g'
  }
  method gist { self.Str }

Now all we really have to do is handle the addition:

multi infix:<+> (Mass \a, Mass \b) { Weight.new: a.grams + b.grams }
multi infix:<-> (Mass \a, Mass \b) { Weight.new: a.grams - b.grams }

This works great, but does automatically convert everything back to being a g unit even if both were kg, etc.  At this point, it's a bit of taste.  You could decide that the first or second unit has precedence, or implement some more complex logic (larger or smaller, or the one that results in no more than three decimal places or no more than three whole units, etc).  I'll leave that for you to implement.

As I said, this would work great as a module.  You can do that like this:

unit module SI-Operators;

my constant \si-prefix := Map.new(
  'Y',  10 **  24,   'y',  10 ** -24,
  'Z',  10 **  21,   'z',  10 ** -21,
  'E',  10 **  18,   'a',  10 ** -18,
  'P',  10 **  15,   'f',  10 ** -15,
  'T',  10 **  12,   'p',  10 ** -12,
  'G',  10 **   9,   'n',  10 **  -9,
  'M',  10 **   6,   'μ',  10 **  -6,
  'k',  10 **   3,   'm',  10 **  -3,
  'h',  10 **   2,   'c',  10 **  -2,
  'da', 10 **   1,   'd',  10 **  -1,
  '',   1
);

# MASSES

class Mass is export (:mass) {
  has $.grams;
  has $.prefix;
  method new  ($number, $prefix = '') {
    self.bless:
      :grams($number * si-prefix{$prefix}),
      :$prefix
  }
  method Str {
    $!grams / si-prefix{$!prefix}
    ~ $!prefix
    ~ 'g'
  }
  method gist { self.Str }
}

multi infix:<+> (Mass \a, Mass \b) is export (:mass) { Mass.new: a.grams + b.grams }
multi infix:<-> (Mass \a, Mass \b) is export (:mass) { Mass.new: a.grams - b.grams }

multi postfix:<Yg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'Y'  }
multi postfix:<Zg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'Z'  }
multi postfix:<Eg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'E'  }
multi postfix:<Pg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'P'  }
multi postfix:<Tg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'T'  }
multi postfix:<Gg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'G'  }
multi postfix:<Mg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'M'  }
multi postfix:<kg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'k'  }
multi postfix:<hg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'h'  }
multi postfix:<dag> (Numeric() $m) is export (:mass) { Mass.new: $m, 'da' }
multi postfix:<g>   (Numeric() $m) is export (:mass) { Mass.new: $m       }
multi postfix:<dg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'd'  }
multi postfix:<cg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'c'  }
multi postfix:<mg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'm'  }
multi postfix:<μg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'μ'  }
multi postfix:<ng>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'n'  }
multi postfix:<pg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'p'  }
multi postfix:<fg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'f'  }
multi postfix:<ag>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'a'  }
multi postfix:<zg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'z'  }
multi postfix:<yg>  (Numeric() $m) is export (:mass) { Mass.new: $m, 'y'  }

# LENGTH

class Length is export (:length) {
  has $.meters;
  has $.prefix;
  method new  ($number, $prefix = '') {
    self.bless:
      :meters($number * si-prefix{$prefix}),
      :$prefix
  }
  method Str {
    $!meters / si-prefix{$!prefix}
    ~ $!prefix
    ~ 'm'
  }
  method gist { self.Str }
}

multi infix:<+> (Length \a, Length \b) is export (:length) { Length.new: a.meters + b.meters }
multi infix:<-> (Length \a, Length \b) is export (:length) { Length.new: a.meters - b.meters }

multi postfix:<Ym>  (Numeric() $l) is export (:mass) { Length.new: $l, 'Y'  }
multi postfix:<Zm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'Z'  }
multi postfix:<Em>  (Numeric() $l) is export (:mass) { Length.new: $l, 'E'  }
multi postfix:<Pm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'P'  }
multi postfix:<Tm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'T'  }
multi postfix:<Gm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'G'  }
multi postfix:<Mm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'M'  }
multi postfix:<km>  (Numeric() $l) is export (:mass) { Length.new: $l, 'k'  }
multi postfix:<hm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'h'  }
multi postfix:<dam> (Numeric() $l) is export (:mass) { Length.new: $l, 'da' }
multi postfix:<m>   (Numeric() $l) is export (:mass) { Length.new: $l       }
multi postfix:<dm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'd'  }
multi postfix:<cm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'c'  }
multi postfix:<mm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'm'  }
multi postfix:<μm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'μ'  }
multi postfix:<nm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'n'  }
multi postfix:<pm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'p'  }
multi postfix:<fm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'f'  }
multi postfix:<am>  (Numeric() $l) is export (:mass) { Length.new: $l, 'a'  }
multi postfix:<zm>  (Numeric() $l) is export (:mass) { Length.new: $l, 'z'  }
multi postfix:<ym>  (Numeric() $l) is export (:mass) { Length.new: $l, 'y'  }

What's great about doing it as a module is you can only import what you need.  For example, if in most of your program you're not using SI units, there's no reason to polute the operator namespace with lots of postfixes.  But when you do:

sub stuff-with-lengths {
  use SI-Operators :length;
  say 25mm + 78km # 78000.025m
}

Note that multiplying, etc, would need you to implement more math operator subs.  But could also lead to interesting things like a Length * Length generating an Area etc, and Area * Length getting Volume


LeetCode: Deepest Leaves Sum C# Given a binary tree, return the sum of values of its deepest leaves.

  Constraints:

  The number of nodes in the tree is between 1 and 10^4. The value of
  nodes is between 1 and 100.

Please review for performance and style

using GraphsQuestions;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace TreeQuestions
{
    /// <summary>
    /// https://leetcode.com/problems/deepest-leaves-sum/
    /// </summary>

    [TestClass]
    public class DeepestLeavesSumTest
    {
        [TestMethod]
        public void ExampleTest()
        {
            var root = new TreeNode(1);
            root.left = new TreeNode(2);
            root.left.left = new TreeNode(4);
            root.left.right = new TreeNode(5);
            root.left.left.left = new TreeNode(7);
            root.right = new TreeNode(3);
            root.right.right = new TreeNode(6);
            root.right.right.right = new TreeNode(8);
            DeepestLeavesSumClass deepest = new DeepestLeavesSumClass();
            Assert.AreEqual(15, deepest.DeepestLeavesSum(root));
        }
    }

    public class DeepestLeavesSumClass
    {

        int maxDepth = 0;
        int sum = 0;
        public int DeepestLeavesSum(TreeNode root)
        {
            if (root == null)
            {
                return 0;
            }
            DFS(root, 0);
            return sum;
        }

        private void DFS(TreeNode root, int depth)
        {
            if (root == null)
            {
                return;
            }
            if (maxDepth < depth + 1)
            {
                maxDepth = depth + 1;
                sum = root.val;
            }
            else if (depth + 1 == maxDepth)
            {
                sum += root.val;
            }
            DFS(root.left, depth + 1);
            DFS(root.right, depth + 1);
        }
    }

}

Rename DFS to DeepestLeavesSum.

Extract  depth + 1 to variable.

In here else if (depth + 1 == maxDepth) the else is reduant.

The order of depth + 1 and maxDepth is different between the Ifs. One time  maxDepth is on the left side and other time on the right side.

Usually, when writing recursion, you return the value and not store it in the class.

If you call DeepestLeavesSum twice with the same object the second call will return wrong results because it will use sum and maxDepth of the first call. A good practice is avoiding saving state.


Stack Implementation in Python I have written a simple stack implementation and would like some feedback as to what I could improve in my code and coding practices.

# A simple stack in python with ints.
class stack():
    def __init__(self, maxSize=16):
        self.maxSize = maxSize
        self.data = []

    def isEmpty(self):
        if len(self.data) == 0:
            return True
        else:
            return False

    def isFull(self):
        if len(self.data) == self.maxSize:
            return True
        else:
            return False

    def push(self, data):
        if not self.isFull():
            self.data.append(data)
            return "OK"
        else:
            return "ERR_Stack_Full"

    def pop(self):
        if not self.isEmpty():
            output = self.data[len(self.data) -1]
            del self.data[len(self.data) -1]
            return output, "OK"
        else:
            return "ERR_Stack_Empty"

    def npop(self, n):
        output = []
        if len(self.data) >= n:
            for i in range(n):
                output.append(self.pop()[0])
            return output, "OK"

Based on the input given here is my modified code (Sorry about the entire thing being indented, the code sample button was being difficult):

    class EmptyStackError(Exception):
        def __init__(self):
                super().__init__("Stack is empty: cannot pop from an empty stack!")

    class FullStackError(Exception):
            def __init__(self):
                    super().__init__("Stack is full: cannot push to a full stack!")

    # A simple stack in python with ints.
    class Stack():
        def __init__(self, max_size=16):
            self.max_size = max_size
            self.data = []

        def is_empty(self):
            if len(self.data) == 0:
                return True

        def is_full(self):
            if len(self.data) == self.max_size:
                return True

        def push(self, data):
            if not self.is_full():
                self.data.append(data)
                return data
            else:
                raise FullStackError()

        def pop(self):
            if not self.is_empty():
                output = self.data[len(self.data) -1]
                del self.data[len(self.data) -1]
                return output
            else:
                raise EmptyStackError()

Better to follow a common stack API

The common API for a stack:

push(item): add an item on the top of the stack and then return the item
pop(): remove the top item from the stack and return it

My objection is against what your methods return.
push returns a message, pop returns an (item, message) tuple.
If you think about it, the message is useless.
Users of this class would have to check the output of each call,
and evaluate success or failure using string comparison.
This is very weak.

Instead of using the return value to indicate success and failure,
it would be better to follow the common API,
and raise exceptions in case of failures.
For example like this:

class EmptyStackError(Exception):
    def __init__(self):
        super().__init__("Stack is empty: cannot pop an empty stack")

class StackFullError(Exception):
    def __init__(self):
        super().__init__("Stack is full")

class stack():
    # ...

    def push(self, data):
        if self.isFull():
            raise StackFullError()
        self.data.append(data)
        return data

    def pop(self):
        if self.isEmpty():
            raise EmptyStackError()
        item = self.data[len(self.data) -1]
        del self.data[len(self.data) -1]
        return item

Python conventions

Follow PEP8, the official coding style guide of Python. For example:

The convention for class names is PascalCase, so stack should be Stack
The convention for method names is snake_case, so isFull should be is_full


Abstracted email sender with content providers I've created an email sender with providers for the email content, that will be changed based on the email type. I need some help to enhance it.
These two models are used to send
public class EmailAddress
{
    public string Name { get; set; }
    public string Address { get; set; }
}

public class EmailMessage
{
    public EmailMessage()
    {
        ToAddresses = new List<EmailAddress>();
        CcAddresses = new List<EmailAddress>();
    }

    public List<EmailAddress> ToAddresses { get; set; }
    public List<EmailAddress> CcAddresses { get; set; }
    public string Subject { get; set; }
    public string Content { get; set; }
}

The content provider provides all the email information(subject, body, To and Cc)
public interface IEmailContentProvider
{
    EmailMessage Message { get; }
}

Then we have the abstracted email sender IEmailSender that has a single method Send which uses IEmailContentProvider parameter to get the email information
interface IEmailSender
    {
        Task Send(IEmailContentProvider provider);
    }

I have an example for the content provider WelcomEmailProvider
public class WelcomEmailProvider : IEmailProvider
{
        public EmailMessage Message { get; }

        public WelcomEmailProvider(string address, string name)
        {
            Message = new EmailMessage
        {
            Subject = $"Welcome {name}",
            Content = $"This is welcome email provider!",
            ToAddresses = new List<EmailAddress> { new EmailAddress { Address = address, Name = name} }
        };
    }
}

The IEmailSender implementation:
public class EmailSender : IEmailSender
{
    private readonly SmtpOptions _options;

    public EmailSender(IOptions<SmtpOptions> options)
    {
        _options = options.Value;
    }

    public async Task Send(IEmailContentProvider provider)
    {
        var emailMessage = provider.Message;
        var message = new MimeMessage();
        message.From.Add(new MailboxAddress(_options.Sender.Name, _options.Sender.Address));
        message.To.AddRange(emailMessage.ToAddresses.Select(x => new MailboxAddress(x.Name, x.Address)));
        message.Cc.AddRange(emailMessage.CcAddresses.Select(x => new MailboxAddress(x.Name, x.Address)));

        message.Subject = emailMessage.Subject;
        message.Body = new TextPart(TextFormat.Html) { Text = emailMessage.Content };

        using var emailClient = new SmtpClient();
        await emailClient.ConnectAsync(_options.Server, _options.Port, _options.EnableSsl);
        await AuthenticatedData(emailClient);

        await emailClient.SendAsync(message);
        await emailClient.DisconnectAsync(true);

    }

    private async Task AuthenticatedData(SmtpClient smtpClient)
    {
        if (string.IsNullOrWhiteSpace(_options.Username) || string.IsNullOrWhiteSpace(_options.Password))
            return;

        emailClient.AuthenticationMechanisms.Remove("XOAUTH2");
        await emailClient.AuthenticateAsync(_options.Username, _options.Password);
    }
}

And here is, how to use it and send an email:
class Sample
{
    private readonly IEmailSender _emailSender;
    public Samole(IEmailSender emailSender)
    {
        _emailSender = emailSender;
    }

    public async Task DoSomethingThenSendEmail()
    {
        await _emailSender.Send(new WelcomEmailProvider("someone@example.com", "Someone"));
    }
}

public EmailMessage()
{
    ToAddresses = new List<EmailAddress>();
    CcAddresses = new List<EmailAddress>();
}

Not necessary, I can understand if you just initiate the ToAddress, however, initiating the lists like this might consume a lot of memory (imagine you have large amount of EmailMessage instances! So, I would suggest to keep them as null, and use null validation to force initiating them when it's needed (like in sender).
your overall design is good enough, however, you could do this directly :
public class EmailMessage
{
    public EmailAddress From { get; set; }
    public IEnumerable<EmailAddress> To { get; set; }
    public IEnumerable<EmailAddress> Cc { get; set; }
    public IEnumerable<EmailAddress> Bcc { get; set; }
    public string Subject { get; set; }
    public string Body { get; set; }
}

public interface IEmailProvider
{
    IEmailServerSetting ServerSettings { get; set; }
}

EmailMessage should contain From(required), To(required), CC(optional),BCC(optional), Subject, and Body as a full model of the message. The reason behind that is that it will be always paired together as requirement to any message, later on, it would be also easier to implement on the database side. using IEnumerable<EmailAddress> would open to use any collection that implements IEnumerable. So, it's not restricted to List.
For IEmailProvider As an email provider should also contain its own settings as requirement. This would make things easier for you, to have each email message with it's own settings. This way, you can send multiple emails from different providers with no extra coding. IOptions is treated independently, but in reality, IOptions won't be useful by itself, and it would only used by EmailProvider, so if we add it to the contract, it'll always be implemented with the interface, this way you forced the provider to always have IOptions. Besides, IOptions should be renamed to something like EmailServerSetting or EmailProviderSetting to relate it to the main implementation. As these are settings and not options. Use IOptions to handle the email optional settings and features that can be managed like sending as text or html, disable/enable attachments, pictures ..etc.
Also, you need a middle-ware class to wrap things up, and uses SmtpClient. This would give you the advantage of increasing the flexibility of your current work and wrap them under one roof along with ease things up for reusing code (such as MimeMessage, TextPart ..etc.) instead of reimplement it on each new sender. It also would give you the ability to create a collection to store multiple providers if you're going that far. also, you will be able to add new providers, handle them, handle messages, and keep your work scoped.
Here is how I imagine the final usage :
Creating a new provider :
// creating a new email provider
    public class SomeEmailProvider : IEmailProvider
    {
        // only set the settings internally but it's exposed to be readonly
        public EmailServerSetting ServerSettings { get; private set; }

        public SomeEmailProvider()
        {
            //set up the server settings
            ServerSettings = new EmailServerSetting
            {
                ServerType = EmailServerType.POP,
                Server = "pop.mail.com",
                Port = 995,
                Encryption = EmailServerEncryption.SSLOrTLS,
                EnableSsl = true
            };
        }
       // some other related code
    }

now, creating a new mail message and sending it :
// can be single object or collection
var messages = new EmailMessage
{
    From = new EmailAddress("Test", "test@mail.com"),
    To = new EmailAddress [] {
         new EmailAddress("Test1", "test1@mail.com"),
         new EmailAddress("Test2", "test2@mail.com")
        },
    Subject = "Testing Subject",
    Body = "Normal Text Body"
};

using(var client = new EmailClient(new SomeEmailProvider()))
{
    client.Options = new EmailClientOptions
    {
        // would send it as plain text
        EnableHtml = false
    };

    client.Messages.Add(messages);

    client.Send();
}


Looking for a better way to handle this growing array used for preg_replace first time visiting Code Review.  I figured this was a more appropriate spot than SO for this question.   Here goes...

Currently, I have a php function that converts bbcode format text in to HTML (among other things).   A code snippet is as follows...

function bbcode_format ($str) {

 $simple_search = array(
    '/\[b\](.*?)\[\/b\]/is',  // bold
    '/\[i\](.*?)\[\/i\]/is',  // italics
    '/\[u\](.*?)\[\/u\]/is',  // underline
)

  $simple_replace = array(
    '$1',
    '$1',
    '$1'
)

    $str = preg_replace ($simple_search, $simple_replace, $str);
    return $str;
}

The code itself works fine.   However, in my actual code, there are currently around 60 items in each array I'm checking for, as I also used it to convert smileys, for example.

My concern is that this approach doesn't seem to be scalable, at least in the sense that the array just keeps growing and growing.

I'm wondering if anyone has another approach as to how this might be done, or any suggestions to improve my current method.   Thanks!

Additional Info

Thanks for the suggestions so far...   I may not have used the best examples above, as b / i / u are only a small set of what I'm replacing.    I probably have more "smiley" replacements, than anything, so those generally look like this...

'/:\)/is',          // smiley :)
'/:d/is',           // laugh smiley :D
'/\:gift1\:/is',    // Holiday Gift 1 :gift1:
'/\:gift2\:/is',    // Holiday Gift 2 :gift2:

Each of those is then replaced with an  tag.

You have two issues: how to represent all of the desired transformations as code, and how to perform the substitutions efficiently.

To address the data representation issue — you want to use an associative array.

$simple_replacement_patterns = array(
    '/\[b\](.*?)\[\/b\]/is' => '$1',
    '/\[i\](.*?)\[\/i\]/is' => '$1',
    '/\[u\](.*?)\[\/u\]/is' => '$1',
);

However, there is a lot of redundancy there, which violates the Don't Repeat Yourself principle.  Ideally, your source code should contain just

$simple_tag_translations = array(
    'b' => 'strong',
    'i' => 'em',
    'u' => 'u',
);

… and the computer build $simple_replacement_patterns for you.

Consider mapping [b]bold text[/b] to bold text, [i]italic text[/i] to italic text, and [u]underlined text[/u] to underlined text.  Not only would the mapping be trivial (BBCode is inspired by HTML designed to be easily translated, after all), you would also not be inferring semantic value into the style.  For example, maybe the BBCode text was italicized because it is a title in a bibliography entry, not because it is to be emphasized.


Checkbox Group Function and Usage In CodeIgniter, I have created function (helper) to populate, store and retrieve checkbox group. I just wonder if I have written code is in proper way and optimized or needs some more finishing?

Function

if ( ! function_exists('checkbox_group') )
{
    function checkbox_group($checkboxes = array(), $name)
    {

    // setting hidden field for null, if no checkbox selected
    echo form_hidden($name.'[]', 'null');

    // start checkbox loop
    foreach($checkboxes as $check => $label_text):

        $checked = FALSE;
        $selects = get_config_row($name);
        if(in_array($check, $selects))
        $checked = TRUE;

        echo '
';
        echo form_label(form_checkbox($name.'[]', $check, $checked, 'id="'.$check.'" class="checkbox"') . $label_text, $check);
        echo '
';
    endforeach;
    }
}

Usage

// checkbox items
$checkboxes = array(
    'noindex'=>'No Index',
    'nofollow'=>'No Follow',
    'noarchive'=>'No Archive',
    'nosnippet'=>'No Snippet'
);

// render checkboxes useing function
checkbox_group($checkboxes, 'indexes');

I haven't worked with CodeIgniter before, so if I say something that argues their standards, please excuse me.

It's such a basic piece of code that there isn't too much to be said. One things I notice right off the bat, your checkbox_group() function has a first parameter with a default value. I'm surprised you haven't done anything about this because it should be throwing you a warning.

  Note that when using default arguments, any defaults should be on the
  right side of any non-default arguments; otherwise, things will not
  work as expected.

As per the PHP Functions page

To better separate the function's business and the view, consider creating an output string and returning it to be echoed. You'd then have:

$output = '';
// setting hidden field for null, if no checkbox selected
$output .= form_hidden($name . '[]', 'null');

// start checkbox loop
foreach ($checkboxes as $check => $label_text) :

    $checked = FALSE;
    $selects = get_config_row($name);
    if (in_array($check, $selects))
        $checked = TRUE;

    $output .= '
';
    $output .= form_label(form_checkbox($name . '[]', $check, $checked, 'id="' . $check . '" class="checkbox"') . $label_text, $check);
    $output .= '
';
endforeach;
return $output;

Now you call:

// render checkboxes useing function
echo checkbox_group($checkboxes, 'indexes');


Resolve double auction orders Given a vector of seller orders (asks) of a certain product (each order having a price) and buyer orders (bids) of the same product, I'm creating a function that resolves the auction by matching the best sellers (lowest asked price) with the best buyers (higher offered price) and then clears the market by selecting a price and 'performing' the transactions between all suitable traders.

I'm experienced as programmer but I'm new to Clojure (or any Lisp), so this question is about the "Clojure way" of doing things so that the resulting code is readable, maintainable and efficient.

(ns trade-routes.core
  (:gen-class))

(defn as-trade-result
  [ask bid]
  (let [bid-price (:price bid) ask-price (:price ask)]
    {:traded (<= ask-price bid-price) :bid bid :ask ask}
    )
)

(defn trade-price-tagger
  [price]
    (fn [trade] (merge trade {:price price}))
  )

(defn bid-price
  [trade]
  (-> trade :bid :price))

(defn average
  [a b]
  (* 0.5 (+ a b)))

(defn resolve-auction
  "Returns a list of trades and unresolved seller asks orders and buyer bids orders.
  The highest asks are matched with the lowest bids until all bids are greater than all asks"
  [asks bids]
  (let [best-asks (sort-by :price asks)
        best-bids (reverse (sort-by :price bids))
        succesful-trades (first (partition-by
            :traded
            (map as-trade-result
                 best-asks
                 best-bids)))
        breakeven-successful-trade (last succesful-trades)
        breakeven-price (average
                          (bid-price breakeven-successful-trade)
                          (bid-price breakeven-successful-trade))]
    (map (trade-price-tagger breakeven-price) succesful-trades)))

(defn random-order
    [min max]
    {:price (+ min (rand (- max min)))}
  )

(defn gen-random-orders
    "Create 'total' random orders with price between min and max"
    [total min max]
   (take total (repeatedly #(random-order min max)))
  )

(def test-asks (gen-random-orders 1000 20 100))
(def test-bids (gen-random-orders 1000 0 25))

(defn -main
  "Just tests with test-asks and test-bids."
  [& args]
  (let [succesful-trades (resolve-auction test-asks test-bids)]
   (println (str "There were " (count succesful-trades) " trades"))))

Formatting

It's common to see let-bindings vertically aligned, one per line. You've done that further down in resolve-auction, but even with just two name-value pairs it can be easier to see the logical parts vertically. The closing brackets (any kind) are also usually on the last line of the form, as opposed to indented back out. So

(defn as-trade-result
  [ask bid]
  (let [bid-price (:price bid) ask-price (:price ask)]
    {:traded (<= ask-price bid-price) :bid bid :ask ask}
    )
)

would more idiomatically be formatted as

(defn as-trade-result
  [ask bid]
  (let [bid-price (:price bid)
        ask-price (:price ask)]
    {:traded (<= ask-price bid-price)
     :ask ask
     :bid bid}))

Semantic indentation

Also, the indentation here with (first (partition-by ending the line:

(let [best-asks (sort-by :price asks)
      best-bids (reverse (sort-by :price bids))
      succesful-trades (first (partition-by
          :traded
          (map as-trade-result
               best-asks
               best-bids)))

would more commonly be written in a way that reads left-to-right, top-to-bottom, in terms of expressions. Here are some examples:

(let [best-asks (sort-by :price asks)
      best-bids (sort-by (comp - :price) bids) ; negated :price gives you (reverse) in a single pass
      successful-trades
      (first
        (partition-by :traded (map as-trade-result best-asks best-bids)))]

      ; or

      successful-trades
      (->> (map as-trade-result best-asks best-bids)
           (partition-by :traded)
           first)

The point here is that the indentation is not just a continuation, but actually shows the code structure. Something in the (bottom) right is expected to flow towards the (top) left.

When you get used to this it makes reading code much faster because it's easy to see what is dependent on what, structure-wise.

Side note: since you only want the first group the partition-by is overkill. Consider take-while:

      successful-trades
      (->> (map as-trade-result best-asks best-bids)
           (take-while :traded))

Expose more of the data

There is a balance here between encapsulating complex logic and making things clear. Someone familiar with Clojure knows instantly what assoc, merge, and so on mean. It's less clear exactly what a trade-price-tagger does until one looks it up. For relatively simple cases like that, using an inline function is idiomatic and arguably more clear. Compare:

(defn trade-price-tagger
  [price]
    (fn [trade] (merge trade {:price price}))
  )
...
(map (trade-price-tagger breakeven-price) succesful-trades)

with

(map #(assoc % :price breakeven-price) succesful-trades)

Similarly, accessors like

(defn bid-price
  [trade]
  (-> trade :bid :price))

(bid-price breakeven-successful-trade)

can be useful for deep objects or large programs, but in something short like this (get-in) could be fine:

(get-in breakeven-successful-trade [:bid :price])

In general I think this is readable and reasonably structured. Nice work as someone new to Clojure/Lisp!

My only other suggestion would be potentially to split up resolve-auction. The first half could be a function (defn get-successful-trades [asks bids]), and resolve-auction could just do the tagging of the breakeven point.


Loading and comparing files containing 3D data I have written a program to compare two 3D raw data equal or different. But I think the time consuming would be reduced if I use the STL. How to improve the program? The complete and varifiable code is showed as follows:

#include <cstdio>
#include <string>
#include <iostream>
#include <assert.h>
#include <cstdlib>

using namespace std;

int LoadSample(std::string path_sample, unsigned char*& raw_data, int* w_h_d) {

    FILE *fp;
    if (  fopen_s(&fp, path_sample.c_str(),"rb") )
        return 1;

    int width = 0;
    int height = 0;
    int depth = 0;

    fread(&width,4,1,fp);
    fread(&height,4,1,fp);
    fread(&depth,4,1,fp);

    w_h_d[0] = width;
    w_h_d[1] = height;
    w_h_d[2] = depth;

    raw_data = (unsigned char*)malloc(width*height*depth*sizeof(unsigned char));
    fread(raw_data,height*width*depth,1,fp);
    fclose(fp);

    return 0;
}

int CompareData(unsigned char*& data_train, int* train_w_h_d, unsigned char*& data_test, int* test_w_h_d) {

    if (train_w_h_d[0] == test_w_h_d[0] && train_w_h_d[1] == test_w_h_d[1] && train_w_h_d[2] == test_w_h_d[2]) {

        std::cout<< "Size is equal!\n";

        int all_num = train_w_h_d[0]*train_w_h_d[1]*train_w_h_d[2];
        int count = 0;
        for(int i = 0; i < all_num; i++) {
            if(data_train[i] == data_test[i])
                count++;
        }

        if(all_num == count)
            return 1;

    }
    return 0;
}

int main(){

    string root_path = "F:\\SZU\\Projects\\Fetal_Face\\data\\";
    string filepath_train = root_path + "train\\";
    string filepath_test = root_path + "test\\detect_data\\";

    unsigned char* data_train = NULL;
    unsigned char* data_test = NULL;

    int num_train = 127;
    int num_test = 80;
    int train_w_h_d[3] = {0, 0, 0};
    int test_w_h_d[3] = {0, 0, 0};
    for (int i = 0; i < num_train; i++)
        for (int j = 1; j <= num_test; j++) {

            char num_i[10], num_j[10];
            _itoa_s(i, num_i, 10);
            _itoa_s(j, num_j, 10);
            std::string file_train = filepath_train + "data_" + num_i + ".raw";
            std::string file_test = filepath_test + "data_" + num_j + ".raw";

            if( LoadSample(file_train, data_train, train_w_h_d) )
                std::cout<< "Error: load train data.\n";

            if ( LoadSample(file_test, data_test, test_w_h_d) )
                std::cout<< "Error: load test data.\n";

            if( CompareData(data_train, train_w_h_d, data_test, test_w_h_d) ) {
                std::cout<< "Same data: " << i << "-- train\t\t" << j << " -- test\n"<<std::endl;
                system("PAUSE");
            }
            else{
                std::cout<< i << "-- train\t\t" << j << " -- test is difference." <<std::endl;
            }

            delete[] data_train;
            data_train = NULL;
            delete[] data_test;
            data_test = NULL;
    }

    return 0;
}

The code can run correctly but I want to improve the efficiency by the STL. My compiler is VS2008.

So, how to revise it? By the way, what and where is potential error or something be able to develop in my code.

For inequality, it is sufficient to have one difference. If one difference is found, it's unnecessary to compare more data. There's no point in counting equal data entries and compare that number to the total number of entries, because a single inequality will reduce that number making it impossible to be the same as the total number of entries.

if(data_train[i] == data_test[i])
            count++;

Should be

if(data_train[i] != data_test[i])
            return 0;


MVC Controller Class Here is my controller class for my MVC pattern with all the includes removed.  From what I know of the MVC pattern it is good to have a small controller class.  Mine is just a collection of public static functions which accept requests and send data to the client.

Regarding send().  I'm just wrapping the echo command for sending text from an Ajax call.  This is for consistency and to have all text exiting the same point in code.

I have a ControlEntry (Snippet 2) which help decouple Super Globals and direct entry into the Control, so the two kind of work in Tandem.

Pull_Pic just pulls a path and inserts it into HTML for display.

Snippet 1

<?php

    class Control
    {
        public static function ajax($ajax_type)
        {
            Session::start();
            switch($ajax_type)
            {
                case 'ControlBookmark_add':
                    $control_bookmark_instance = new ControlBookmark('remove');
                    $control_bookmark_instance->add();
                    break;
                case 'ControlBookmark_delete':
                    $control_bookmark_instance = new ControlBookmark('remove');
                    $control_bookmark_instance->delete();
                    break;
                case 'ControlTweet_add':
                    $control_tweet_instance = new ControlTweet('remove');
                    $control_tweet_instance->add();
                    break;
                case 'ControlSignIn':
                    $control_signin_instance = new ControlSignIn('remove');
                    $control_signin_instance->invoke();
                    break;
                case 'ControlSignUp':
                    new ControlSignUp();
                    break;
                case 'ControlTryIt':
                    new ControlTryIt();
                    break;
                case 'ControlSignOut':
                    Session::finish();
                    new ControlSignOut();
                    break;
                default:
                    throw new Exception('Invalid ajax_type');
            }
        }
        public static function reload() // Because reload may be triggered by File Upload (second control path)
        {
            if (session_id() == "")
            {
                Session::start();
            }
            if((Session::status())==='bookmarks')
            {
                include 'class.ViewBookmarks.php';
            }
            else
            {
                include 'class.ViewMain.php';
            }
        }
        public static function upload($fileType, $fileName)
        {
            Session::start();
            $fileInstance = new UploadedFile($fileType, $fileName);
            $fileInstance->createImages();
            Session::updatePic();
            self::reload();
        }
        public static function send($textStream)
        {
            echo $textStream;
        }
    }

**Snippet 2**

    new ControlEntry();

    class ControlEntry
    {
        public function __construct()
        {
            if(isset( $_FILES['ufile']['tmp_name']) )
            {
                if( ( $_FILES['ufile']['tmp_name']) != '' )
                    {
                    Control::upload( $_FILES['ufile']['type'], $_FILES['ufile']['tmp_name'] );
                    }
                else
                    {
                    Control::reload();
                    }
            }
            else if( isset($_POST['ajax_type']) )
            {
                Control::ajax( $_POST['ajax_type'] );
            }
            else
            {
                Control::reload();
            }
        }
    }

It looks like you are trying to write object oriented code, however this is completely procedural. Remove static everywhere and use dependency injection if you want to make this OO.

Static code is hard to test. It is the equivalent of a namespaced function.

Even your new objects that you create are the equivalent of calling a function.

        case 'ControlBookmark_add':
            new ControlBookmark('add');
            break;

I'd recommend one of the following:

Spend a long while learning how to write real object oriented code.
Give up and convert to procedural code. Skipping all of the wasted classes and new's

Some people may disagree that I would suggest moving away from object oriented code, but your code is so far away from object oriented that it makes sense to me.

Response to the Edit

It does look a bit better now.  It would be a good experience for you to unit test this with PHPUnit.  I think you would find it very hard with the static and new.  That would give you a motivation to change it further.  However, I'm guessing the testing this code will receive will be based on seeing the page on the browser, which is okay for a lot of people.

One comment on the edited version:

// Do you really need the 'remove' parameter to your constructor?
// Especially when you call the add method on it?
// Also, objects aren't that special I wouldn't bother with _instance.
$control_bookmark = new ControlBookmark();


Python program for a simple calculator I have written a program for a simple calculator that can add, subtract, multiply and divide using functions.

Here is my code:

# This function adds two numbers
def add(x, y):
   return x + y

# This function subtracts two numbers
def subtract(x, y):
   return x - y

# This function multiplies two numbers
def multiply(x, y):
   return x * y

# This function divides two numbers
def divide(x, y):
   return x / y

print ("Select operation.")
print ("1. Add")
print ("2. Subtract")
print ("3. Multiply")
print ("4. Divide")

choice = input("Enter choice (1/2/3/4): ")

num1 = int(input("Enter first number: "))
num2 = int(input("Enter second number: "))

if choice == '1':
   print(num1, "+", num2, "=", add(num1,num2))

elif choice == '2':
   print(num1, "-", num2, "=", subtract(num1,num2))

elif choice == '3':
   print(num1, "*", num2, "=", multiply(num1,num2))

elif choice == '4':
   print(num1, "/", num2, "=", divide(num1,num2))
else:
   print("Invalid input")

So, I would like to know whether I could make this code shorter and more efficient.

Also, any alternatives are greatly welcome.

Any help would be highly appreciated.

You have several functions. What if you will have 100 functions? 1000? Will you copy-paste all your code dozens of times? Always keep in mind the DRY rule: "Don't repeat yourself". In your case you can store all functions and its info in some kind of structure, like dict.
You run your program, it calculates something once and quits. Why not letting the user have many calculations? You can run the neverending loop with some break statement (old console programs, like in DOS, usually quitted on Q).

Here is the improved code:

# This function adds two numbers
def add(x, y):
    return x + y

# This function subtracts two numbers
def subtract(x, y):
    return x - y

# This function multiplies two numbers
def multiply(x, y):
    return x * y

# This function divides two numbers
def divide(x, y):
    return x / y

print("Select operation.")
print("1. Add")
print("2. Subtract")
print("3. Multiply")
print("4. Divide")

functions_dict = {
    '1': [add, '+'],
    '2': [subtract, '-'],
    '3': [multiply, '*'],
    '4': [divide, '/']
}

while True:
    choice = input("Enter choice (1/2/3/4) or 'q' to quit: ")
    if choice == 'q':
        break
    elif choice in functions_dict:
        num1 = int(input("Enter first number: "))
        num2 = int(input("Enter second number: "))
        print('{} {} {} = {}'.format(
            num1,
            functions_dict[choice][1],
            num2,
            functions_dict[choice][0](num1, num2)
        ))
    else:
        print('Invalid number')


Parzen window density estimation in MATLAB This was my C# implementation of Parzen Window Density Estimation.

The following is my implementation of the same in Matlab:

parzen.m

function [retval] = parzen (matrix, dataPoint, variance)
    [r c] = size(matrix);
    A = ones(r, c).*dataPoint;
    sub = matrix - A;
    up = sub.^2;
    dw = 2 * variance;
    firstPart = 1/(sqrt(2*pi*variance));
    retval = firstPart * exp((-1)*(up/dw));

What do you think about this implementation? Is it correct?

Indentation is automatically fixed in MATLAB by marking everything, and do Ctrl+I.

The common practice is to have the first level alligned all the way to the left. Click here to see a screenshot from the builtin MATLAB function ind2sub. I have made no edits to it.

matrix is not a very good variable name. I suggest you choose something else. I don't have a suggestion off the top of my head, but there are probably more descriptive names.

[r c] = size(matrix);

There's not much more to say, best practice is to separate output values with commas.

The following subtracts the value of datapoint from A. I'm assuming datapoint is a scalar, not a matrix.

[r c] = size(matrix);
A = ones(r, c).*dataPoint;
sub = matrix - A;

This can be simplified a lot. If you want to subtract one value from all elements of a matrix in MATLAB, you can just do:

sub = matrix - dataPoint;

If dataPoint is a matrix, of the same dimensions as matrix, then you can do:

sub = matrix - dataPoint

Yes, that was exactly the same as above. As long as the dimensions match, or one of the variables is a scalar, you can simply subtract one from the other.

I guess dw means down? If so, call it down, it's easier to understand, and is not very hard to write.

You mix camelCase and lowercase. I suggest you stick to one, and do dataPoint and returnValue.

You forgot end in the end. It works without it, but it is supposed to be there.

  Is it correct?

I don't know. I don't know what it's supposed to do. I guess the simplest way to check it is to compare it to your C# implementation.


How to exactly work with the JSON Object and jQuery, adding info to posts I have been working with AJAX and JSON for quite some years now, but to be honest I still do not know exactly what all the functionality are, and how to create optimized code for jQuery.

So my first question: Does anyone know a good tutorial, or better even a cheatsheet where are the functionalities are explained. I have Googled and looked at the normal jQuery documentation for years now, but I still can't manage to think in optimized paterns, if that makes any sense at all.

Now to illustrate this with a relevant problem. Currently I am developing a small public plugin that adds various counters to posts, nothing special here.

&nbsp;

&nbsp;

&nbsp;

&nbsp;

&nbsp;

&nbsp;

Now this has to be filled. The best way I could make this work is with the following json sting:

{ "posts" :
    { "60" :
        { "x" : "100", "y" : "200" , "z" : "300" }
    },
    { "64" :
        { "x" : "400", "y" : "500" , "z" : "560" }
    }
}

Now as for the ajax there is a double loop:

    // AJAX blah blah blah above
success : function( data ){
    posts = data.posts;
    jQuery.each( posts, function( post_id, post_counts ) {
        jQuery.each( post_counts, function( item_name, item_count ) {
            jQuery( '#container-' + post_id + ' .counter-' + item_name ).text( item_count );
        });
    });
}

Now this works fine, but I bet there is a better way for doing this. Maybe change the json string to a more optimized version for doing things quicker.

jQuery has so many special functions.

Anyone have any suggestions.

I don't think that I can give reasons why this is better (i.e. faster).  Only I feel that it is more readable and scalable.

I like to use the data attributes for information like ids.  For a long time, classes were the way to go but we had to deal with cluttered classes :(.  Data attributes keep things cleaner in my opinion–plus it makes it easier to have multiple counters on the page.

x

y

z

x

y

z

Using the same data:

var data = {
    "posts": {
        "60": { "x" : "100", "y" : "200" , "z" : "300" },
        "64": { "x" : "400", "y" : "500" , "z" : "560" }
    }
};

And then using javascripts for-in:

var posts = data.posts;

for(var item in posts) {
    for(var counter in posts[item]) {
        var selector = "[data-counter=" + item + "-" + counter + "]",
            count = posts[item][counter];

        $(selector).text(count);
    }
}


Speeding up loop-rich Matlab function to calculate temperature distribution I would like to speed up this function as much as possible in Matlab.
This is part of a bigger simulation project, and as it is one of the most called functions within the simulation, this is crucial.

For now, I tried generating a MEX file, but the speed was not better.

Vectorizing seems difficult (but would be beneficial due to the nested loops), given the non-linear operations.

What I'm trying to compute with this is the degree of temperature mixing between water layers, given by these equations:

$$\begin{align}
T_j =&\ r\ T_{j+1} + (1-r)\ T_j\quad&\textrm{for } j \in [i,\ i+\Delta i]\\
T_{j+1} =&\ (1-r)\ T_{j+1} + r\ T_j
\end{align}$$

and \$n\$ is the total number of water layers. This must be calculated for all \$j\$, this is for all the layers, at all timesteps.

function y = mixing(T,dis,rr,n)
%% ===================================================================
%  input: temperature of cells array T, distance array dis, number of cells
%         n, mixing ratio r
%
%  output:  new temperature array
%
%  purpose: calculates the temperature array of next timestep
%  ===================================================================

for j = 1:n
    i = 1;
    r = rr;
    while i < dis(j)+1 && j+i <= n
        if (dis(j) < i)
            r = r*(dis(j)-floor(dis(j)));
        end
        d = T(j+i-1);
        T(j+i-1) = r*T(j+i) + (1-r)*T(j+i-1);
        T(j+i) = r*d + (1-r)*T(j+i);
        i = i + 1;
    end
end

y = T;
end

Inputs: T is a 10 × 1 double, dis is a 10 × 1 double, rr is a 1 × 1 double, and n is a 1 × 1 integer value.

First, using vectors would probably speed it up quite much, and would be worth the effort.

However, here's a small tip, that could improve the performance a little bit:

In many of the CPU-hungry iterative computations, you could replace the computations that occur several times by a variable.

In the code, there are several "j+i" terms. They can simply be replaced by a variable named, "jpi". So, the number of additions in each value of j would decrease by 5 (not 6, as you still need to do an addition, once per each iteration of j).

dis(j) is also frequently used, and it might be useful to create another dummy variable, like dis_j = dis(j) once per each iteration of j. I don't know if matlab compiler does this itself to optimize performance. this one stops the cpu from looking at the value of dis(j) each time it is called. So it should help a little bit.


Array based queue implementation with C++20 I implemented a queue that holds elements on an array as underlying data type:
#include <cstddef>
#include <cassert>
#include <concepts>
#include <stdexcept>
#include <type_traits>
#include <iostream>

namespace mine {
template <std::semiregular T, std::size_t N> class queue {
public:
  using value_type = std::remove_cv_t<T>;
  using const_value_type = std::add_const_t<value_type>;

  using reference_type = std::add_lvalue_reference_t<value_type>;
  using const_reference_type = std::add_const_t<reference_type>;

  using size_type = std::size_t;
  using const_size_type = std::add_const_t<size_type>;

  void enqueue(const value_type val) {
    if (current_size == N) {
      throw std::overflow_error("queue overflowed.");
    }

    if (current_size == 0) {
      m_head = m_tail = 0;
    }

    m_tail = current_size;
    m_data[current_size++] = val;
  }

  value_type dequeue() {
    if (current_size == 0) {
      m_head = m_tail = 0;
      throw std::underflow_error("queue underflowed.");
    }

    --current_size;

    auto ret = m_data[m_head];
    m_data[m_head] = T{};
    ++m_head;
    return ret;
  }

  [[nodiscard]] reference_type head() { return m_data[m_head]; }
  [[nodiscard]] const_reference_type head() const { return m_data[m_head]; }

  [[nodiscard]] reference_type tail() { return m_data[m_tail]; }
  [[nodiscard]] const_reference_type tail() const { return m_data[m_tail]; }

  [[nodiscard]] bool empty() noexcept { return current_size == 0; }
  [[nodiscard]] const bool empty() const noexcept { return current_size == 0; }

  [[nodiscard]] size_type size() noexcept { return current_size; }
  [[nodiscard]] const size_type size() const noexcept { return current_size; }

private:
  size_type m_head = 0;
  size_type m_tail = 0;
  int current_size = 0;
  value_type m_data[N]{};
};

}

And I wonder about the possible blunders, what is missing and how to make it better in general. Thanks a lot.

Avoid unnecessary copies
enqueue() takes its parameter by value, which means the caller has to make a copy. Then it has to store that value in the array, making another copy. You can avoid copies by replacing the function that takes a value with two overloads, one for const references and one for rvalue references:
void enqueue(const_reference_type val) {
    // same as before
    ...
}

void enqueue(value_type &&val) {
    // mostly the same
    ...
    // but now we move:
    data[current_size++] = std::move(val);
}

The first version will make just one copy, the second one will use move assignment, which might avoid an expensive copy.
Similarly, dequeue() makes a copy. You can avoid this by splitting this function in two; one that returns a reference to the head element so the caller can read it without having to copy it, and another function that just removes the head element. You already have a head(), so then just modify dequeue() to be:
void dequeue() {
    // mostly the same as before, except without returning anything
    ...
}

This is also why std::queue provides a separate front() and pop() function. Also, doing it this way makes the requirements for T less strict; it now only requires it to be std::default_constructible and std::movable.
Make it look like std::queue
Programming is much easier if everything has a similar interface. Consider renaming your member functions to match that of std::queue:

enqueue() -> push()
dequeue() -> pop()
head() -> front()
tail() -> back()

You could also add emplace() and swap() to make it almost a drop-in replacement.


PHP - Optimizing Recursive Method Here is a recursive function that I wrote to search a file-system to a child depth defined by $depth.

Could it be optimized more?

function stringSearch($working_dir,&$results_array,$depth)
{
    global $search_string;
    global $dir_count;
    global $file_count;
    global $working_url;
    global $max_depth;

    if($max_depth>=($depth+1))
    {
        $dir_array = glob($working_dir.'/*', GLOB_ONLYDIR);
        foreach($dir_array as $new_path)
        {
            stringSearch($new_path,$results_array,($depth+1));
            $dir_count++;
        }
    }

    $handle = opendir($working_dir);
    while ($file = readdir($handle))
    {
        if(is_file($working_dir.'/'.$file))
        {
            if(stripos(str_replace('_',' ',$file),$search_string))
                $results_array[] = array('file'=>$file,'path'=>$working_dir,'url'=>$working_url.str_replace($_SERVER["DOCUMENT_ROOT"],'',$working_dir));

            $file_count++;
        }
    }

    return;
}

I answered this moments ago, but same applies

https://stackoverflow.com/questions/8108175/php-usleep-to-prevent-script-timeout/8108229#8108229

If you simply use : $handle = opendir($working_dir); you can find yourself waiting for the script to timeout on non existing directories. So atleast use:

if (! $handle = opendir($working_dir))
  exit(0);

Its generally way faster to just check if functions are even possible before you start them, although it might feel like your implementing way to many if statements, but if statements are fast and most errors are not ;)


Creating chat box with comet I'm writing a chat application.  I would like to know if there are problems in this code.

This function sends message:

function sendMessage()
{
    var text = $('#messageBox').val();
    $('#messageBox').val('');
    if($('.context').scrollTop() + $('.context').innerHeight() >= $('.context')[0].scrollHeight)
    {
        $(".context").animate({ scrollTop: $('.context')[0].scrollHeight}, 1000);
    }
    $('.context').append('
'+ text +'
');
    if(text != '')
    {
        $('#send').attr('disabled', 'disabled');
        $.ajax({
            url: 'http://localhost/chat/display',
            async: true,
            type: "POST",
            data: 'text=' + text,
            cache: false,
            //dataType: 'json',
            success: function (msg) {
                if (msg == 1)
                {
                    $('#send').removeAttr('disabled');
                }
            },
            error: function (msg) {
                alert('app has an error!');
            }
        });
    }
}

When the user sends a message, the message is written on the page before saving.

This code comes before the AJAX call.  Is it right?

$('.context').append('
'+ text +'
');

This function shows messages:

$(document).ready(function () {
    $(window).load(displayMessage(lastTimestamp));
});

function displayMessage(time)
{
    $.ajax({
        url: 'http://localhost/chat/display',
        async: true,
        type: "POST",
        data: 'time=' + time,
        cache: false,
        dataType: 'json',
        success: function (msg) {
            if(msg)
            {
                if($('.context').scrollTop() + $('.context').innerHeight() >= $('.context')[0].scrollHeight)
                {
                    $(".context").animate({ scrollTop: $('.context')[0].scrollHeight}, 1000);
                }
                $('.context').append(msg[0]);
                time = msg[1];
            }
            setTimeout(displayMessage, 2000, time);
        },
        error: function (msg) {
            displayMessage(time);
        }
    });
}

After the page loads, this function will be run to send data to be displayed.

Display method:

public function display()
    {
        if(isset($_POST['text']))
        {
            $text = DB::Escape($_POST['text']);
            $time = time();
            DB::Query("INSERT INTO `messages` (`user_id`, `to_message_id`, `text`, `timestamp`) VALUES ('{$_SESSION['id']}', '1', '{$text}', '{$time}')");
            if(DB::AffectedRows() > 0)
            {
                return 1;
            }
            else
            {
                return DB::LastError();
            }
        }

        if(isset($_POST['time']))
        {
            $time = intval($_POST['time']);
            $current = time();
            while (time() - $current < 10)
            {
                $result = self::findAll("WHERE (`timestamp` > '{$time}' AND `user_id` != '{$_SESSION['id']}')");
                if(count($result) > 0)
                {
                    $html = '';
                    foreach ($result as $obj)
                    {
                        $html .= '
'. $obj->text .'
';
                    }
                    return json_encode(array($html, $obj->timestamp));
                }
                else
                {
                    sleep(3);
                }
            }
        }
    }

Note this section:

while (time() - $current < 10)

This code slows sending and receiving messages so it takes several seconds to send and insert a message when I press Enter or click the sending button.

I'm using last timestamp for selecting messages from database. Is there a problem as the same time in timestamp column? I'd like to fix this apparent performance problem.

This code comes before the AJAX call. Is it right?

I would suggest adding the message to the DOM via the success callback - that way, if an error occurs then the message isn't displayed on the page.

Other feedback

Javascript

Cache DOM lookups

I would suggest you cache DOM references - e.g. store $('#messageBox'), $('#send'), $('.context') in variables (or better yet, constants if you are using ecmascript-6), preferably in a DOM-ready callback, because DOM-lookups aren't exactly cheap....

Accessing elements by class name can return multiple

Also: $('.context') is equivalent to document.getElementsByClassName()... which returns multiple elements. While methods like .scrollTop() and .innerHeight() operate for the first matched element, it would be more appropriate to select an element by id (i.e. with an id selector).

$(document).ready() with nested $(window).load()

I don't believe it is necessary to add the window load callback within the DOM ready callback. Also, bearing in mind that you posted this code a couple years ago, .load() is now deprecated as of jQuery version 1.8, because there is a new method .load() in the AJAX section.

Note that in the line below, displayMessage() is called and does not return a function, so the call to $(window).load() appears superfluous:

$(window).load(displayMessage(lastTimestamp));

To really make that function displayMessage an onload handler, one could make a partially applied function using Function.bind() :

$(window).load(displayMessage.bind(null, lastTimestamp));

PHP

Separate endpoints

It would be wise to separate the code in the display function into two different endpoints - one for fetching messages (which should perhaps use the GET HTTP verb), and one for creating a new post (which can stay as a POST or a PUT request). That way display isn't concerned with creating messages, so as to adhere to the Separation of Concerns principle.

Laravel code?

It looks like some of the Laravel DB facade and Eloquent methods (e.g. findAll()) are used, which makes me believe this is a Laravel controller method. If that is the case, it would be wise to access POST data via the Request object. Then lines like

$text = DB::Escape($_POST['text']);

Can be updated to use the Request::input() method:

$text = DB::Escape($request->input('text'));

Or using the dynamic input style:

$text = DB::Escape($request->text);


A character array of arbitary length with 'R', 'B', 'W' is needed to be sorted in that order There is a char array of n length.  The array can have elements only from
  any order of R, B, W. You need to sort the array so that order should R,B,W
  (i.e. all R will come first followed by B and then W).

  Constraints: Time
  complexity is O(n) and space complexity should be O(1).

  Assumption:
  You can assume one swap method is given with signature swap(char[]
  arr, int index1, int index2) that swaps number in unit time.  Method
  given to implement: public sort(char[]array);

Here is my implementation of it.  A better solution from anyone is appreciated. Anyone is free to point out any mistakes.

public static void sort(char[] arr){
     int rIndex = 0, wIndex = arr.length -1;
     for (int i = 0 ; i <= wIndex;){
         if ( arr[i] == 'R' ){
             swap(arr, i , rIndex ++ );
             i ++;
         } else if (arr[i] == 'W' ){
             swap(arr, i , wIndex -- );
         }else{
            i ++;
         }
     }
}

Don't bother swapping.  Just do a counting sort!

public static void sort(char[] arr) {
    // Count occurrences of each letter
    int r = 0, b = 0, w = 0;
    for (char c : arr) {
        switch (c) {
          case 'R': r++; break;
          case 'B': b++; break;
          case 'W': w++; break;
          default: throw new IllegalArgumentException();
        }
    }

    // Write out the appropriate repetitions of each letter
    int i = 0;
    while (r-- > 0) arr[i++] = 'R';
    while (b-- > 0) arr[i++] = 'B';
    while (w-- > 0) arr[i++] = 'W';
}

Not only is the code easy to understand, it's also gentle to the cache since you always proceed linearly down the array.


Function figuring out which close-by value comes from which input value I would like to optimize the below traceback function. This function is part of an elementary step of the program and is called a lot...

import numpy as np

def traceback(tuple_node, tuple_node_alt):
    """
    Compute which value from tuple_node_alt comes from which value from tuple_node.
    Return a dictionnary where the key are the values from tuple_node and the values are the idx at which
    the value may be located in tuple_node_alt.
    """
    # Compute the tolerances based on the node
    tolerances = [0.1 if x <= 100 else 0.2 for x in tuple_node]

    # Traceback
    distance = dict()
    alt_identification = dict()
    for k, x in enumerate(tuple_node):
        distance[k] = [abs(elt-1) for elt in [alt_x/x for alt_x in tuple_node_alt]]
        alt_identification[x] = list(np.where([elt <= tolerances[k]+0.00001 for elt in distance[k]])[0])

    # Controls the identification and corrects it
    len_values = {key: len(val) for key, val in alt_identification.items()}

    if all([x <= 1 for x in len_values.values()]):
        return alt_identification
    else:
        for key, value in alt_identification.items():
            if len(value) <= 1:
                continue
            else:
                other_values = [val for k, val in alt_identification.items() if k != key]
                if value in other_values:
                   continue
                else:
                    for val in other_values:
                        set1 = set(value)
                        intersec = set1.intersection(set(val))
                        if len(intersec) == 0:
                            continue
                        else:
                            alt_identification[key] = [v for v in value if v not in intersec]

    return alt_identification

The input is composed of 2 tuples which do not need to have the same size. e.g.

tuple_node = (40, 50, 60, 80)
tuple_node_alt = (87, 48, 59, 39)

The goal is to figure out which value from tuple_node_alt may come from which value from tuple_node. If the value from tuple_node_alt is within a 10% margin from a value from tuple_node, it is considered that it comes from this value.

e.g. 39 is within a 10% margin of 40. It comes from 40.
This aprt is perform in the "Traceback" section, where a distance dictionnary is computed and where the idx are computed. With the example above, the output is:

Out[67]: {40: [3], 50: [1], 60: [2], 80: [0]}

However, because of a potential overlapping of the tolerance band, 3 scenarios exists:

Scenario 1: each value has been identified to one alternative value. That's the case above.

Scenario 2:

tuple_node = (40, 50, 60, 80)
tuple_node_alt = (42, 55, 54)

55 and 54 are both in the tolerance band of both 50 and 60. Thus, the output is:

Out[66]: {40: [0], 50: [1, 2], 60: [1, 2], 80: []}

Scenario 3:

tuple_node = (40, 50, 60)
tuple_node_alt = (42, 55, 59)

This is when the control part comes in play. With this input, alt_identification becomes: Out[66]: {40: [0], 50: [1], 60: [1, 2], 80: []}. However, the 55 can not come from 60 since 50 only has one possibility: 55. Thus, this number being already taken, the correct output which is provided through the control & correct section is:

Out[66]: {40: [0], 50: [1], 60: [2], 80: []}

I would really like to optimize this part and to make it a lot more quicker. At the moment, it takes:

# With an input which does not enter the control & correct part.
node = (40, 50, 60, 80)
node_alt = (39, 48, 59, 87)
%timeit traceback(node, node_alt)
22.6 µs ± 1.04 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

# With an input which need correction
node = (40, 50, 60, 100)
node_alt = (42, 55, 59, 89)
%timeit traceback(node, node_alt)
28.1 µs ± 1.88 µs per loop (mean ± std. dev. of 7 runs, 10000 loops each)

A couple of low-hanging fruit inefficiencies:

distance = dict().  The distance[k] value is computed in a loop, and only every used in the next statement of the loop.  It does not need to be stored in a dictionary.
all([ ...list comprehension... ]): You are using list comprehension to build up a list, which you immediately pass to all(...).  There is no need to actually create the list.  Just use all(...list comprehension...).
set1 = set(value).  This is inside a for val in other_values: loop, where value and set1 are not changed.  Move the statement out of the for loop, to avoid recreating the same set each iteration.
len_values is only used in the afore mentioned all(...), and only the the values of len_values dictionary are used.  As such, the len_value dictionary construction is also unnecessary, and the if statement can be written:

if all(len(val) <= 1 for val in alt_identification.values()):

Since you are returning alt_identification from the if statement, and after the if...else statement, you can invert the test, and remove one return statement:

if any(len(val) > 1 for val in alt_identification.values()):
    for key, value in alt_identification.items():
        # ... omitted for brevity ...

return alt_identification

Similarly, the two if condition: continue else: could be re-written if not condition:.

Other possible improvements:

tolerances[k] is only used in next for k loop.  The list can be removed and the calculations move into the loop.
numpy is only used for a list(np.where([...])[0]) operation, which is fairly obfuscated.  A simple list comprehension can be used instead.
The values of alt_identification are of type list, and converted (repeatedly) into a set() in the "control & correct" code.  They could be stored as set() to avoid repeated conversions.

Here is my rework of the code, with the changes based on above comments:

def traceback(tuple_node, tuple_node_alt):

    def close_alternates(x):
        tolerance = (0.1 if x <= 100 else 0.2) + 0.00001
        return set( k for k, alt_x in enumerate(tuple_node_alt)
                    if abs(alt_x/x - 1) <= tolerance )

    alt_identification = { x: close_alternates(x) for x in tuple_node }

    if any(len(val) > 1 for val in alt_identification.values()):
        for key, values in alt_identification.items():
            if len(values) > 1:
                other_values = [val for k, val in alt_identification.items() if k != key]
                if values not in other_values:
                    for other in other_values:
                        alt_identification[key] -= other

    return alt_identification

I'm getting up to a 2.8x speedup with the above code, on your test data set that require correction.