Basic singlepage blog application I've started learning Mithril framework, which looks pretty awesome. I've created a basic blog app and have a few questions about code style and structure.

var app = {};

app.PostList = function(list) {
  return m.prop(list || []);
};

app.vm = {};

app.Post = function(data) {
  var data = data || {};
  this.id = m.prop(data.id);
  this.title = m.prop(data.title);
  this.content = m.prop(data.content);
};

app.vm.init = function() {
  var self = this;
  self.posts = new app.PostList();
  m.request({method: 'GET', url: '/posts', background: true, type: app.Post})
    .then(self.posts)
    .then(m.redraw);
};

app.controller = function() {
  app.vm.init();
};

app.view = function(ctrl) {
  return m('body', [
      m('h1', 'My blog'),
      m('ul',
        app.vm.posts().map(function(el){
          return m('li', [
            m('a[href="/posts/'+el.id()+'"]', {config: m.route}, JSON.stringify(el))
          ]);
        })
      ),
      m('a[href="/posts/add/"]', {config: m.route}, 'add new post')
  ]);
};

var add = {};

add.newPost = function(data) {
  data = data || {};
  this.title = data.title || '';
  this.content = data.content || '';
};

add.controller = function() {
  var post = new add.newPost();
  this.post = post;
  this.create = function(e) {
    e.preventDefault();
    m.request({
      method: 'POST',
      url: '/posts',
      data: post
    }).then(function() {m.route('/');});
  };
};

// data binding helper function
// http://lhorie.github.io/mithril-blog/asymmetrical-data-bindings.html
function formBinds(data) {
  return function(e) {data[e.target.name] = e.target.value;};
}

add.view = function(ctrl) {
  return m('body', [
    m('form', {
      onsubmit: ctrl.create,
      onchange: formBinds(ctrl.post)
    }, [
      m('a[href="/"]', {config: m.route}, 'All Posts'),
      m('br'),
      m('input[type=text][name=title][placeholder="post title"][required]'),
      m('br'),
      m('textarea[name=content][placeholder="post content"][required]'),
      m('br'),
      m('input[type=submit][value=Post]')
    ])
  ]);
};

var details = {};

details.vm = {};
details.vm.init = function(postId) {
  var self = this;
  self.comment = {
    postId: postId,
    name: '',
    body: ''
  };
  self.post = new app.Post();
  m.request({method: 'GET', url: '/posts/'+postId, type: app.Post}).then(function(data) {
    data = data[0];
    self.post = data;
  });
  self.comments = m.request({method: 'GET', url: '/posts/'+postId+'/comments'});

  self.add = function(e) {
    e.preventDefault();
    if (self.comment.body) {
      var newComment = self.comment;
      // TODO: add request error handler (how to?)
      m.request({method: 'POST', url: '/posts/'+postId+'/comments', data: newComment}).then(function() {
        var comments = self.comments();
        comments.push(newComment);
        self.comments(comments);
      });
    }
  }.bind(self);
};

details.controller = function() {
  var postId = +m.route.param('postId');
  this.postId = postId;
  details.vm.init(postId);
};

details.view = function(ctrl) {
  return m('body', [
    m('a[href="/"]', {config: m.route}, 'All posts'),
    m('h1', details.vm.post.title()),
    m('p', details.vm.post.content()),
    m('form', {
      onsubmit: details.vm.add,
      onchange: formBinds(details.vm.comment)
    }, [
      m('input[type=text][name=name][placeholder="your name"][required]'),
      m('br'),
      m('textarea[name=body][placeholder="comment body"][required]'),
      m('br'),
      m('input[type=submit][value=Post]')
    ]),
    m('ul', details.vm.comments().map(function(comment) {
      return m('li', comment.name+': '+comment.body);
    }))
  ]);
};

//setup routes to start w/ the `#` symbol
m.route.mode = 'hash';

//define a route
m.route(document, '/', {
    '/' : app,
    '/posts/add': add,
    '/posts/:postId': details
});

You can find the complete project on my GitHub.
Please, provide some advice or comments about how to implement it with Mithril.

I noticed you're using app as a module name (which is understandable, since it's what the tutorial in the Mithril site does now). For real apps, you should try to name your modules more descriptively. For example, if the module displays a list of Posts, you might want to call it posts or postList.

Instead of calling m.request from vm, I normally pull the request out into a method on the model class, for example:

var Post = function(data) {
  var data = data || {};
  this.id = m.prop(data.id);
  this.title = m.prop(data.title);
  this.content = m.prop(data.content);
}
Post.list = function() {
  return m.request({method: 'GET', url: '/posts', type: app.Post})
}

You might optionally want to wrap m.request itself in another method, e.g.

var req = function(args) {
  return m.request(args)
}
/*...*/
Post.list = function() {
  return req({method: 'GET', url: '/posts', type: app.Post})
}

This lets you easily configure requests globally (e.g. if you need to set HTTP headers on all requests)

On a side note, I generally don't recommend using the background:true + m.redraw combo (especially on first render), unless you have a really good reason. When you do that, you're allowing your view to run early, which means all of your asynchronous data needs to be considered nullable, which in turn means you need more defensive code in the view. It's also a band-aid solution if the root of the problem is a slow web service.

Re: this comment: "TODO: add request error handler (how to?)"

Assuming you created a model method for posting as well:

Post.save = function(args) {
  return m.request({method: 'POST', url: '/posts', data: args, unwrapError: unwrap})
}

Then you can use the rejection handler of the promise to bind errors to a getter-setter

self.error = m.prop("")
self.add = function() {
  /*...*/
  Post.save()
    .then(null, self.error)
}

You can define the unwrapError function to drill down into a response to get the desired error message string out of the JSON structure. (There's also the extract option which lets you handle the response at the XHR level)

So, if your response looks like {msg: "something died"}, then you could write unwrapError: function(e) {return e.msg}, and then to display the error, you would simply call vm.error() in the view, which would return the "something died" string.

I also normally use a layout wrapping module, which looks something like this:

var layout = {}

layout.controller = function(module) {
  this.controller = new module.controller
  this.view = module.view
}

layout.view = function(ctrl) {
  return m(".layout", [
    ctrl.view(ctrl.controller)
  ])
}

layout.wrap = function(routes) {
  var map = {}
  Object.keys(routes).map(function(r) {
    map[r] = {
      controller: function() {
        return new layout.controller(routes[r])
      },
      view: layout.view
    }
  })
  return map
}

Then I can wrap modules like this:

m.route(document, "/", layout.wrap({
  "/": posts
}))

With this, the inner modules don't need to redefine the layout stuff all the time.

Regarding usage of vm versus using the controller, you only really need view-models if you need your modules to be globally accessible by other modules (this usually only happens when you have a deep hierarchy of sub-templates).

Most of the time, using fat controllers yield shorter, clearer code than using view-models (see app.vm.init + app.controller vs add.controller).

Still on the topic of controllers, one technique that I've seen in angular styleguides is to code controllers like this:

foo.controller = function() {
  var foo = {bar: 1}
  return {
    foo: foo
    doStuff: doStuff
  }

  function doStuff() {
    alert(foo.bar)
  }
}

This style avoids issues with this scoping and makes it easier to scan the public signature of the controller.

When using m.route, I recommend that you bind to document.body instead of document, so that you can put stylesheets in <head> and take advantage of the browser downloading/rendering optimizations.

Other than those, the code looks fairly well organized. The views are well written, and the code is easy to read, overall.


CodeForces: Creating the Contest I wrote a solution to this question. The task is to take an input of n increasing positive integers (1 ≤  n ≤ 2∙105), each entry as large as 109, and find the length of the longest subsequence where each element is no more than double the previous element.

Example input

10
1 2 5 6 7 10 21 23 24 49

Example output

4

… because the longest subsequence is [5, 6, 7, 10].

My solution works for small numbers, but exceeds the time limit for big inputs.

#include<iostream>
#include<vector>
#include<algorithm>
using namespace std;
int main()
{

 int n,a,c=1,max_count=1;
 cin>>n;
 vector<int>v;
 for(int i=0;i<n;i++)
 {
     cin>>a;
    v.push_back(a);
}
for(int i=0;i<n;i++)
{
    int l=i+1;
    while((l<n)&&(v[l]<=2*v[i]))
    {
        c++;
        l++;
        max_count=max(max_count,c);
    }

    c=1;
}
cout<<""<<max_count;
return 0;
}

There is no need to store the entries in a vector. There is also no need for a nested loop.  This solution will run in O(n) time and O(1) space.

Technically, int is only guaranteed to hold up to 16 bits. Use long to ensure that you can accommodate the limits.

Also, please avoid using namespace std;. You can drop the useless cout<<"".

#include <algorithm>
#include <iostream>

int main() {
    long n, a, seq_len = 0, longest = 0;
    std::cin >> n;
    for (long prev_a = 0; n--; prev_a = a) {
        std::cin >> a;
        if (a <= 2 * prev_a) {
            seq_len++;
        } else {
            seq_len = 1;
        }
        longest = std::max(longest, seq_len);
    }
    std::cout << longest << '\n';
}


PHP Image size recalculation function I got a function to recalculate the image size based on two settings.

Force ratio, force the requested ratio, default the original ratio will be used.
Keepsmaller, the uploaded image may never become bigger

If keep smaller is not enabled, by default we will focus calculation on the first reached, it's hard to explain see enclosed excel screen you will probebly understand.

<?php
function recalc($oldWidth, $oldHeight, $reqWidth, $reqHeight, $forceRatio = false, $keepSmaller = false){
    if ($keepSmaller) {
        $maxWidth   = ($oldWidth < $reqWidth)   ? $oldWidth  : $reqWidth;
        $maxHeight  = ($oldHeight < $reqHeight)  ? $oldHeight : $reqHeight;
    } else {
        $maxWidth   = $reqWidth;
        $maxHeight  = $reqHeight;
    }

    $ratio      = ($forceRatio) ? ($reqWidth / $reqHeight) : ($oldWidth / $oldHeight);

    if ($ratio != ($maxWidth / $maxHeight))
    {
        if ($keepSmaller && $ratio > ($maxWidth / $maxHeight)){
            $maxHeight = $maxWidth / $ratio;
        } elseif (!$forceRatio && $ratio < ($maxWidth / $maxHeight)) {
            $maxWidth  = $maxHeight * $ratio;
        }
    }

    echo $maxWidth . ', ' . $maxHeight;
}

recalc( 480,  360, 1440, 1080); echo "
";
recalc(1920, 1440, 1440, 1080); echo "
";
recalc( 480,  360, 1920, 1080); echo "
";
recalc(3840, 2880, 1920, 1080); echo "
";
echo "
";

recalc( 480,  360, 1440, 1080, false, true); echo "
";
recalc(1920, 1440, 1440, 1080, false, true); echo "
";
recalc( 480,  360, 1920, 1080, false, true); echo "
";
recalc(3840, 2880, 1920, 1080, false, true); echo "
";
echo "
";

recalc( 480,  360, 1440, 1080, true); echo "
";
recalc(1920, 1440, 1440, 1080, true); echo "
";
recalc( 480,  360, 1920, 1080, true); echo "
";
recalc(3840, 2880, 1920, 1080, true); echo "
";
echo "
";

recalc( 480,  360, 1440, 1080, true, true); echo "
";
recalc(1920, 1440, 1440, 1080, true, true); echo "
";
recalc( 480,  360, 1920, 1080, true, true); echo "
";
recalc(3840, 2880, 1920, 1080, true, true); echo "
";
echo "
";

This are the input, params and outputs used for testing.
However some scenarios are still unwatched.

The 1st table states itself
the second contains the original sizes
the third the requested/target sizes
the last one is what 'we' expect for it to become

Complexity

It looks to me that even with this simple function, the complexity inside takes you for a ride. You don't have it under control. The reason is obvious: You try to do too much in one function.

If your program can set two flags, it can also choose between different functions: One for simple resizing, one for resizing while keeping the aspect ratio the same, etc. As you can guess there could be some overlap between these function. Therefore:

Classes

Classes are the ideal structures to put these multiple functions in, we call them 'methods', and let them share some common code. Let me give an example, that does some of the things that your function does, and some it doesn't:

function greatestCommonDivisor($int1,$int2)
// return greatest common divisor, euclidean algorithm
{
   if ($int2 == 0) return $int1;
   else return greatestCommonDivisor($int2,$int1 % $int2);
}

class ImageHandle
{
  private $outputSize;
  private $originalSize;

  public function __construct($width,$height)
  // constructor, set initial size
  {
    $this->setSize($width,$height);
    $this->originalSize = $this->getSize();
  }

  public function getSize()
  // get the Size of the rectangle
  {
    return $this->outputSize;
  }

  public function setSize($width,$height)
  // set the Size of the rectangle
  {
    $this->outputSize = ['width' => $width,'height' => $height];
    return $this;
  }

  public function resetSize()
  // go back to the orginal size
  {
    $this->outputSize = $this->originalSize;
    return $this;
  }

  private function computeAspectRatio($width,$height,$prefix = '')
  // return the aspect ratio as two numbers, with prefix
  {
    $divisor = greatestCommonDivisor($width,$height);
    $name    = ($prefix == '' ? 'aspect' : $prefix.'Aspect');
    return [$name.'X' => intdiv($width,$divisor),
            $name.'Y' => intdiv($height,$divisor)];
  }

  public function getAspectRatio()
  // return the aspect ratio as two numbers for set size
  {
    extract($this->getSize());
    return computeAspectRatio($width,$height);
  }

  public function setAspectRatio($newWidth,$newHeight)
  // set a new aspect ratio while keeping within the current width and height
  {
    extract($this->getSize());
    extract(computeAspectRatio($newWidth,$newHeight,'new'));
    if ($newAspectX > $aspectX) $height = $width*$newAspectY/$newAspectX;
                           else $width  = $height*$newAspectX/$newAspectY;
    return $this->setSize($width,$height);
  }

  public function rescaleSize($maxWidth,$maxHeight)
  // rescale, while keeping aspect ratio the same
  {
    extract($this->getSize());
    $newWidth  = $maxWidth;
    $newHeight = round($maxWidth*$height/$width);
    if ($newHeight > $maxHeight) {
      $newWidth  = round($newWidth*$maxHeight/$newHeight);
      $newHeight = $maxHeight;
    }
    return $this->setSize($newWidth,$newHeight);
  }

  public function echoInfo($title)
  // just here for this example, should be removed
  {
    echo $title.': ('.implode(',',$this->getSize()).
                 ') '.implode(':',$this->getAspectRatio()).'
';
    return $this;
  }

}

$test = [[ 480,  360, 1440, 1080],
         [1920, 1440, 1440, 1080],
         [ 480,  360, 1920, 1080],
         [3840, 2880, 1920, 1080]];

foreach ($test as $data)
{
  $image = new ImageHandle($data[0],$data[1]);
  $image->echoInfo('Orginal')
        ->setSize($data[2],$data[3])
        ->echoInfo('Set')
        ->resetSize()
        ->rescaleSize($data[2],$data[3])
        ->echoInfo('Rescale')
        ->resetSize()
        ->setAspectRatio($data[2],$data[3])
        ->echoInfo('Aspect Ratio');
  echo '
';
}

When ran this returns:

Orginal: (480,360) 4:3
Set: (1440,1080) 4:3
Rescale: (1440,1080) 4:3
Aspect Ratio: (480,360) 4:3

Orginal: (1920,1440) 4:3
Set: (1440,1080) 4:3
Rescale: (1440,1080) 4:3
Aspect Ratio: (1920,1440) 4:3

Orginal: (480,360) 4:3
Set: (1920,1080) 16:9
Rescale: (1440,1080) 4:3
Aspect Ratio: (480,270) 16:9

Orginal: (3840,2880) 4:3
Set: (1920,1080) 16:9
Rescale: (1440,1080) 4:3
Aspect Ratio: (3840,2160) 16:9

Ok, when you're not used to classes this might seem like killing a fly with an atomic bomb. I agree. But bear with me. I didn't call this class ImageHandle for nothing. You could extend this class to include other methods, mine are just examples, or an image loader and image saver, and much more.

However, notice the relative simplicity of each method: They don't get overburdened with functionality, so they stay relatively simple. Each one performs a single task.

Just like in your function, you could apply more than one method to change the size, but I found that to be a bit useless. Nevertheless, it will work.

PS: I intentionally left out the comments I normally put in my classes, this example is already quite long. You don't need them anyway, it isn't rocket science. :-)

PS2: I made this class slightly harder to understand, because I'm now using an array to store width and height. Some methods get very short, which is nice, but in others I use extract() to get at the data in the array. This is a method I often use. Arrays can be useful, but if you're not used to them they make it harder to read the code. Sorry for that.


Creating an array length n * n, separating it into n groups, and adding numbers to it n times I am completing the CodingBat exercises for Java. Here is the one I have completed just now:

Given n > = 0, create an array length n * n with the following pattern, shown here for n = 3 :
{0, 0, 1,    0, 2, 1,    3, 2, 1} (spaces added to show the 3 groups).

Here is my code:
    public int[] squareUp(int n){

    int[] ary = new int[n * n];
    int numberToInsert = 1;
    int count = 0;
    int target = n;

    //Guard condition for n = 1
    if (n == 1) {
        ary[0] = 1;
        return ary;
    }

    //Do the following n times
    for (int i = 0; i < n; i++) {
        //Working from the end of the array backwards, insert the number
        //at the appropriate point
        for (int j = ary.length - numberToInsert; j > 0; j -= n) {
            ary[j] = numberToInsert;
            //The amount of times a number is inserted decreases each
            //iteration
            count++;
            if (count == target) {
                count = 0;
                target--;
                break;
            }
        }
        numberToInsert++;
    }
    return ary;
}

My questions are:

How would I implement this without the guard condition? The result without it is 0 because it doesn't get as far as the second for loop, as numberToInsert is already 1. I tried to set this as 0, and increment it later in the code, but I couldn't figure this out.
What alternative is there to the count and target system I implemented? Was this a good or bad way to achieve the outcome?
How else could the numbers be inserted? The method I used felt the most logical but I'm curious to see other methods.

Addressing into a 1d or 2d array is not a real problem for a square array

ary[i,j] = value;

can be written as

ary[i*n + j] = value

where n is the dimension.

The following passes the tests on CodingBat though there may be more elegant ways to approach it

public int[] squareUp(int n) {
    int[] result = new int[n*n];
    int zeroCount = n-1;
    for(int outer = 0; outer <n; outer++){
        for(int inner = 0; inner < n; inner++){
            result[(outer * n) + inner] = zeroCount > inner
                                            ? 0
                                            : (n-inner);
        }
        zeroCount--;
    }
    return result;
}


Beginner python solution to CSES "Permutations" I'm working through the CSES problem set as I learn to program, and this solution was accepted, but I'm not quite happy with it. I just know there's a faster way to check whether or not a solution's valid instead of looping through every element, and I'm sure there're plenty more improvements that could be made. Any feedback is appreciated!
Problem: A permutation of integers 1,2,…,n is called beautiful if there are no adjacent elements whose difference is 1.
Given n, construct a beautiful permutation if such a permutation exists.
If there are several solutions, you may print any of them. If there are no solutions, print "NO SOLUTION".
def check_solution(sol):
    last = sol[0]
    for num in sol:
        if num - last == 1 or num - last == -1:
            return False
        last = num
    return True

a = [int(x) for x in range(1, int(input())+1)]
b = []
even = a[1::2]
odd = a[::2]

b.extend(even)
b.extend(odd)

if check_solution(b):
    print(*b)
else:
    print("NO SOLUTION")

Your list slicing idea is a good one. You could simplify further by doing something
analogous with ranges, eliminating most of the need for intermediate variables
def beautiful_permutation(n):
    odd  = list(range(1, n + 1, 2))
    even = list(range(2, n + 1, 2))
    return even + odd

Your validator probably should check more things and can also be simplified by
taking advantage of abs(). Optionally, you could express the adjacent-number
check using all() and zip(). We're still looping over the numbers, but
just more concisely.
def check_solution(n, sol):
    return (
        # Must not be empty.
        sol and
        # Must cover all intergers 1 through N.
        set(range(1, n + 1)) == set(sol) and
        # No adjacent numbers with difference of 1.
        all(abs(a - b) != 1 for a, b in zip(sol, sol[1:]))
    )

One additional point. The method used to create the sequence nearly ensures
validity. By definition, the individual sub-lists (odd and even) will follow
the adjacent-number rule. The only way there can be a problem is at the
junction point where we glue them together. But our function could directly
validate the issue. If you were to add this and include a preliminary check for
bad or special inputs (0, 1, etc.), you would have a function that,
at least for practical purposes, is provably correct -- no need for a validator!?!
if odd and even and abs(even[-1] - odd[0]) != 1:
    return even + odd
else:
    return None


Making this password search more intelligent In a homework, there is an exercise in which I have a script that is used to search for a password.

One of the questions is if is possible to make it "more intelligent", and I'm stuck on it.

#!/bin/bash

space1="a b c d e f g h i j k l m n o p q r s t u v w x y z"

if [  $# -le 1 ]
then
    echo "Ussage: " $0 SALT PASSWORD_CODED
    exit
fi

for i in $space1
do
for j in $space1
do
for k in $space1
do

    variable=$(openssl passwd -crypt -salt "$1" "$i$j$k")
    if [ "$variable" = $2 ]
        then
        echo password found: $i$j$k
        exit
    fi
done
done
done

If it is possible, I prefer only a clue to help me to discover the answer correct, not the answer itself. I promise to post the result when I get the answer for myself.

There's no real way to speed up the brute-force enumeration of all possible 3-letter passwords.

Perhaps you could use the dictionary.  There's a finite list of 3-letter English words.    They may be slightly more common.

Also, if you google for  "most common passwords", some kind of 3-letter version of that list could be tried before anything else.


Random number wrapper class I made this class to handle any min max integer value and return a random number in the entered parameter range. I tested it to work with all combinations(I think). But is this totally wrong or written with unnecessary amounts of code? Are there obvious Java convention violations or obvious redundancy in this?

I was wondering if it was possible to do the same without instantiating Random, since I have read that object instantiation is more resource demanding than method invoking, like invoking Math.random(). I just couldn't make that work, unfortunately, as I didn't save the strange non-working code that that ended with.

I tried the solution here. However, I don't understand what the rand() part is, which is too bad since it seemed really simple with just one line of code.

abstract class RandomInteger {
     static int randomNumber;

     public static int returnRandomIntRange(int start, int end){

         if(end < start){
             throw new IllegalArgumentException("Start cannot exceed End.");
         } else if(end == 0 && start == 0){
             throw new IllegalArgumentException("Start and End can't both be 0.");
         } else if(end == start){
             throw new IllegalArgumentException("Start and End can't be the same.");

         } else if(end >= 0 && start <= 0){
             Random random=new Random();
             int range;
             range = end - start + 1;
             System.out.println("aEnd > 0 && aStart < 0. 1range is: " + range);
             randomNumber=(random.nextInt(range))+(start);
             System.out.println("aEnd > 0 && aStart >= 0. invoked. 1randomNumber is: " + randomNumber);

         } else if(end > 0 && start > 0){
             Random randomGenerator=new Random();
             int range;
             range = end - start + 1;
             System.out.println("aEnd > 0 && aStart >= 0. invoked. 2range is: " + range);
             randomNumber = randomGenerator.nextInt(range) + start;
             System.out.println("aEnd > 0 && aStart >= 0. invoked. 2randomNumber is: " + randomNumber);

         } else if(end < 0 && start < 0){
             Random randomGenerator=new Random();
             int range;
             range = (start - end -1) * -1;
             System.out.println("aEnd <= 0 && aStart < 0. invoked. 3range is: " + range);
             randomNumber = ((randomGenerator.nextInt(range)+ start));
             System.out.println("aEnd <= 0 && aStart < 0. invoked. 3randomNumber is: " + randomNumber);
         }

         return randomNumber;
     }

}

That does look a little confusing.  The example you looked at is for C, not Java.  I would do this:

int random = (int) Math.floor(Math.random() * (max - min + 1) ) + min;

It works like this:  Math.random() returns a double between 0 and 1 (although never actually equal to 1).  The full range of values you want is (max - min + 1) (the +1 is because you probably want max to min inclusive), so this will scale the number over the correct number of integer values.  We floor it (round down) because we want an int, and finally shift it upwards by adding min to put the numbers in the correct range.  We also cast to an int since we don't want it as a double.

EDIT: On further inspection, this is better; no casting and rounding of doubles:

Random rand = new Random();
int random = rand.nextInt(max - min + 1) + min;


Creating a file of md5 hashes for all files in a directory in PowerShell I have been trying to write the md5 hashes for all files in a directory and its subdirectories to a file. Ideally, replicating the output of the Unix command find . -type f -exec md5sum {} + (i.e. two columns: lowercase hashes and relative file paths [with forward slashes] separated by a space and terminated only by a line feed).

With a lot of help from Mark Wragg, LotPings and others on stackoverflow, the following command appears to compute md5 hashes for all files in a directory and its subdirectories (including those files without file extensions and those with square brackets in the filename).

(Get-FileHash -Algorithm MD5 -LiteralPath (Get-ChildItem -Recurse -File).fullname | ForEach-Object{"{0} {1}" -f $_.Hash.ToLower(),(Resolve-Path -LiteralPath $_.Path -Relative)} | Out-String) -replace '\r(?=\n)' -replace '\\','/' | Set-Content -NoNewline -Encoding ascii $ENV:USERPROFILE\Desktop\hashes.txt

The two uses of -LiteralPath seems to help with filenames containing square brackets and (Get-ChildItem -Recurse -File).fullname gets the full path of all nested files, including those without file extensions. The rest is just formatting.

Can any one tell me where I can find more information about .fullname? I've tried searching for it on Google but without any luck.

I had used Get-ChildItem "*.*" -Recurse, which gives full file paths but only for files with dots in the filename. Whereas, Get-ChildItem "*" -Recurse doesn't always give the full path for some reason (and returns both files and folders). Compare:

Get-ChildItem "*.*" -Recurse | foreach-object { "$_" }

Get-ChildItem "*" -Recurse | foreach-object { "$_" }

The order of entries in the hashes file won't be the same as those from the Unix command but compare-object in PowerShell appears to ignore the order of lines, e.g. (https://serverfault.com/questions/5598/how-do-i-diff-two-text-files-in-windows-powershell)

compare-object (get-content oldHashes.txt) (get-content newHashes.txt)

or

diff (cat oldHashes.txt) (cat newHashes.txt)

I rewrote your code from scratch how I would have written it myself. Maybe it can give you some ideas.

$lines = gci -Recurse -File |
    Get-FileHash -Algorithm MD5 | % {
        $unixPath = $_.Path -replace '\\','/'
        $hash = $_.Hash.ToLower()
        "$hash $unixPath" }

$lines -join "`n" |
    Out-File -Encoding ascii -NoNewline $ENV:USERPROFILE\Desktop\hashes.txt

Notes:

gci is an alias for Get-ChildItem.
I used gci -File to get files only.
% is an alias for ForEach-Object.
I didn't use it, but here is the help for FileSystemInfo.FullName
cat is an alias for Get-Content.

Feel free to ask any questions.


Parsing random phrases from a file This essentially reads a specially-structured file from the scanner and then parses random phrases and prints them:

{
<start>
<greeting> <object>
}

{
<greeting>
hello
bonjour
aloha
}

{
<object>
world
universe
multiverse
}

Possible random phrases could be: "hello world", "bonjour universe", etc...

Does anyone have some ideas as to how I might decrease the runtime? I heard you could run your code in parallel using multiple threads but was both unsure exactly how to do this and whether it would help at all? The slightest increase in runtime would be beneficial (having a little competition to see who can do it the fastest).

It is not considered cheating so long as I do the majority of the programming, so please just point me in the right direction and provide suggestions.

package comprehensive;

import java.io.File;
import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.Random;
import java.util.Scanner;

/**
 * Generates random phrases by reading the specified grammar file
 * and outputting the number of phrases specified by the user.
 * @author Jun Tang and John Newsome
 *
 */
public class RandomPhraseGenerator {

    public static HashMap<String,ArrayList<String>> map;

    public static void main(String[] args) throws FileNotFoundException{
        Scanner k = new Scanner(new File(args[0]));
        int numPhrases = Integer.parseInt(args[1]);
        addKeys(k);
        while(numPhrases > 0){
            parseStrings();
            numPhrases--;
        }
    }

    /**
     * Assimilates all of the keys and their associated values into map (a public static hashmap).
     * @param scan The scanner that is reading from the specified file.
     */
    public static void addKeys(Scanner scan){
        map = new HashMap<String, ArrayList<String>>();
        while(scan.hasNextLine()){
            if(scan.nextLine().equals("{")){
                String key = scan.nextLine();
                map.put(key, new ArrayList<String>());
                String next = scan.nextLine();
                while(!next.equals("}")){
                    map.get(key).add(next);
                    next = scan.nextLine();
                }
            }
        }
    }

    /**
     * Parses random String phrases from map and prints them.
     * Must already have assimilated keys and values prior by using the addKeys() method.
     */
    public static void parseStrings(){
        Random random = new Random();
        StringBuilder strBuf = new StringBuilder((map.get("<start>").get(0)));
        int firstInstance  = strBuf.indexOf("<", 0);
        int secondInstance = strBuf.indexOf(">", firstInstance);
        while(firstInstance >= 0 && secondInstance >= 0){
            String nonTerminal = strBuf.substring(firstInstance, secondInstance+1);
            strBuf.replace(firstInstance, secondInstance+1, (map.get(nonTerminal).get(random.nextInt(map.get(nonTerminal).size()))));
            firstInstance  = strBuf.indexOf("<", firstInstance);
            secondInstance = strBuf.indexOf(">", firstInstance);
        }
        System.out.println(strBuf);
    }
}

I'm no expert in performance optimization or even threads, so my code review will mostly be some style suggestions, but I do have suggestions how I would implement the the phrase building to be faster - purely from my gut :-)

A nitpick at the beginning: You should try and clean up your indentations. They are all over the place and make reading the code a bit more difficult
You should declare variables to use interfaces where appropriate instead of concrete class. That makes the code more flexible for example in case you get the keys for a different source. In your case declare your map as Map<String,List<String>>.
Avoid global variables. If would be better to have addKeys return the map (and thus be renamed readKeys) and pass it on to parseString as an argument.
In the main function a for loop is probably more appropriate instead of the while.
Try to get rid of the duplicate scan.nextLine(); in the inner while loop, by moving the exiting condition inside the loop.

Now to optimizing the phrase building. There are two main points I would consider here:

Avoid copying data. Currently you convert your string with the placeholders into a StringBuilder and insert the text in place of the placeholder. Both creating the StringBuilder and inserting into them requires allot of data copying internally.
Don't repeat the parsing of the placeholder text. Instead I would parse the text once into a data structure and then generate the random phrases using that structure.

I could write some (pseudo) code to demonstrate, what I mean, but you should be writing it yourself, so I'll wait and see, if you can come up with an implementation based on those two points.


Introductory HTML exercise I have started learning HTML. Here is one of the very basic HTML pages I have written. I would appreciate advice about how to write code especially regarding indentation.

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
<title> Checking Different Headings | Isnt it fun?</title>
</head>

<body>
<h1>  This is big</h1>
<h2>  This is also good </h2>
<h3>  This is also good </h3>
<h4>  This is also good  </h4>
<h5>  This is also good </h5>
<h6>  This is small </h6>
</body>
</html>

I start my intention in head and body.

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">

    <title> Checking Different Headings | Isnt it fun?</title>

This is big

This is also good

This is also good

This is also good

This is also good

This is small

I also keep my tag tight–no spaces between text and tag.


XML design for parsing in C# I am trying to design XML with configuration info so I can parse it into this code:

...
services.Configure<IdentityOptions>(options =>
{
    // Password settings
    options.Password.RequireDigit = true;
    options.Password.RequiredLength = 8;
    options.Password.RequireNonAlphanumeric = false;
    options.Password.RequireUppercase = true;
    options.Password.RequireLowercase = false;
    options.Password.RequiredUniqueChars = 6;
});
...

I have already 4 versions of the XML code, but I don't know which one to choose:

<identity>
    <!-- version 1 -->
    <password>
        <RequireDigit>true</RequireDigit>
        <RequiredLength>8</RequiredLength>
        <RequireNonAlphanumeric>false</RequireNonAlphanumeric>
        <RequireUppercase>true</RequireUppercase>
        <RequireLowercase>false</RequireLowercase>
        <RequiredUniqueChars>6</RequiredUniqueChars>
    </password>
    <!-- version 2 -->
    <password   RequireDigit="true"
                RequiredLength="8"
                RequireNonAlphanumeric="false"
                RequireUppercase="true"
                RequireLowercase="false"
                RequiredUniqueChars="6"
    />
    <!-- version 3 -->
    <password>
        <RequireDigit value="true"/>
        <RequiredLength value="8"/>
        <RequireNonAlphanumeric value="false"/>
        <RequireUppercase value="true"/>
        <RequireLowercase value="false"/>
        <RequiredUniqueChars value="6"/>
    </password>
    <!-- version 4 -->
    <password>
        <RequireDigit/>
        <RequiredLength value="8"/>
        <RequireUppercase/>
        <RequiredUniqueChars value="6"/>
    </password>
</identity>

Are there any rules when to choose which one?
Or how to decide which is the best or the most suitable?

You are trying to recreate functionality that is already provided out of the box.

Reference Configure an ASP.NET Core App

From documentation it supports providers for INI, JSON, and XML.

  Each configuration value maps to a string key. There's built-in binding support to deserialize settings into a custom POCO object (a simple .NET class with properties).

Reference ASP.NET Core 1.0 Configuration Deep Dive

For XML, you need to add this NuGet package: Microsoft.Extensions.Configuration.Xml and the format that provider uses would look like this for your settings

Note: It matches closely to version 1 in your example

<configuration>
    <identityOptions>
        <password>
            <RequireDigit>true</RequireDigit>
            <RequiredLength>8</RequiredLength>
            <RequireNonAlphanumeric>false</RequireNonAlphanumeric>
            <RequireUppercase>true</RequireUppercase>
            <RequireLowercase>false</RequireLowercase>
            <RequiredUniqueChars>6</RequiredUniqueChars>
        </password>
    </identity>
</configuration>

And the following would be added as part of configuration

//...
.AddXmlFile("appsettings.xml", optional: true, reloadOnChange: true)
//...

If using JSON then

{
  "IdentityOptions": {
    "Password": {
      "RequireDigit": true
      "RequiredLength": 8
      "RequireNonAlphanumeric": false
      "RequireUppercase": true
      "RequireLowercase": false
      "RequiredUniqueChars": 6
    }
  }
}

And the following would be added as part of configuration

//...
.AddJsonFile("appsettings.json", optional: true, reloadOnChange: true)
//...

You appear to already be setting up IOptions, which is good.

//...
// Register the ConfigurationBuilder instance which IdentityOptions binds against.
services.Configure<IdentityOptions>(Configuration);

//...

This will allow constructor dependency injection with IOptions<TOptions> to access settings

private readonly IdentityOptions options;

public MyClass(IOptions<IdentityOptions> identity) {
    options = identity.Value;
}


Get consecutive integer number ranges from list of int This is an algorithm to return a list of consecutive integer ranges from a list of integers.

The practical use of such an algorithm is to filter lists more efficiently, i.e. rather than check e.g. 1000 items (1..1000; x==1 || x==2 || ... || x==1000), it might be possible to check only 2 items (x >= 1 && x <= 1000).

Does this algorithm have any mistakes, can it be optimized, or are there any other improvements you can suggest?

(As a side note: I know this code does not follow the standard C# naming conventions.  I do not like to follow that particular convention; I prefer "snake_case" as it is easier on my eyes.)

Sample output:

(x >= 0 && x <= 1690)
(x >= 13642 && x <= 15331)
(x >= 27283 && x <= 27296)
(x >= 27769 && x <= 27776)
(x >= 28249 && x <= 28256)
(x >= 28729 && x <= 28736)
(x >= 29209 && x <= 29222)

The algorithm (built Visual Studio 2017, C# 7.3, .NET 4.7.2):

public static List<(int from, int to)> get_consecutive_ranges(List<int> fids)
{
    if (fids == null || fids.Count == 0) return null;

    fids = fids.OrderBy(a => a).Distinct().ToList();

    var fids_fast = new List<(int from, int to)>();

    var is_conseq_with_last = true;
    var start_index = 0;
    var end_index = 0;
    for (var fids_index = 0; fids_index < fids.Count; fids_index++)
    {
        var first = fids_index == 0;
        var last = fids_index == fids.Count - 1;
        if (!first && fids[fids_index - 1] == fids[fids_index] - 1)
        {
            is_conseq_with_last = true;
        }
        else if (!first)
        {
            is_conseq_with_last = false;
        }

        if (!is_conseq_with_last && !first && !last)
        {
            end_index = !first && !last ? fids_index - 1 : fids_index;
            fids_fast.Add((fids[start_index], fids[end_index]));
            start_index = fids_index;
        }
        else if (last)
        {
            if (!is_conseq_with_last)
            {
                if (!first)
                {
                    end_index = fids_index - 1;
                }

                fids_fast.Add((fids[start_index], fids[end_index]));
                start_index = fids_index;
            }

            end_index = fids_index;
            fids_fast.Add((fids[start_index], fids[end_index]));
        }
    }

    fids_fast.ForEach(a => Console.WriteLine($"(x >= {a.@from} && x <= {a.to})"));

    return fids_fast;
}

Example use:

// slow:
body = body.Where(a => fids.Contains(a.fid)).ToList();

// fast:
body = body.Where(a => fids_fast.Any(x => a.fid >= x.from && a.fid <= x.to)).ToList();

Your code takes a list, ensures it's sorted and distinct, then assembles and returns another list. This approach is problematic when the input is in fact a generator, in which case your code forces the entire list to memory first. Also, the data might already be sorted or the caller might not be interested in the entire result.

public static List<(int from, int to)> get_consecutive_ranges(List<int> fids)
{
    if (fids == null || fids.Count == 0) return null;

    fids = fids.OrderBy(a => a).Distinct().ToList();

A much more idiomatic approach is to build an extension method that works on IEnumerable and yields its results. The call to Distinct().OrderBy(a => a) should be left to the caller. Also note that Distinct doesn't guarantee any particular order, so the sorting must always happen afterwards. The actual logic is overly complicated in your code and be greatly simplified as well. Here is how I would do it:

public static IEnumerable<(int begin, int end)> Ranges(this IEnumerable<int> nums)
{
    var e = nums.GetEnumerator();
    if (e.MoveNext())
    {
        var begin = e.Current;
        var end = begin + 1;
        while (e.MoveNext())
        {
            if (e.Current != end)
            {
                yield return (begin, end);
                begin = end = e.Current;
            }
            end++;
        }
        yield return (begin, end);
    }
}

Add a simple helper function for pretty printing, which should also be an extension method:

public static string Show(this IEnumerable<(int begin, int end)> ranges)
{
    return "[" + string.Join(",", ranges.Select(r => r.end - r.begin == 1 ? $"{r.begin}" : $"{r.begin}-{r.end-1}")) + "]";
}

Example usage:

Console.WriteLine(new int[] { }.Ranges().Show());                             -> "[]"
Console.WriteLine(new int[] { 1 }.Ranges().Show());                           -> "[1]"
Console.WriteLine(new int[] { 1, 2, 3, 4, 5 }.Ranges().Show());               -> "[1-5]"
Console.WriteLine(new int[] { 1, 2, 3, 5 }.Ranges().Show());                  -> "[1-3,5]"
Console.WriteLine(new int[] { 1, 3, 5, 6 }.Ranges().Show());                  -> "[1,3,5-6]"
Console.WriteLine(new int[] { 1, 3, 3, 3, 5, 6 }.Distinct().Ranges().Show()); -> "[1,3,5-6]"
Console.WriteLine(new int[] { 6, 3, 5, 1 }.OrderBy(i => i).Ranges().Show());  -> "[1,3,5-6]"


Laravel bulk insert optimization I'm currently using a make shift INSERT on DUPLICATE UPDATE function that relies on a DB raw query from eloquent. I'm looking to improve the quality of the code and the speed. Please take a look

function bulkInsertBookTable($data)
{
    $inserts = [];
    foreach ($data as $item) {
        $inserts[] = implode(', ', ['"' . $item['book_id'] . '"', $item['isbn']]);
    }

    $inserts = Collection::make($inserts);

    $inserts->chunk(500)->each(function($ch) {
        $insertString = '';
        foreach ($ch as $element) {
            $insertString .= '(' . $element . '), ';
        }

        $insertString = rtrim($insertString, ", ");

        try {
            DB::insert("INSERT INTO book_datas (`book_id`, `isbn`) VALUES $insertString ON DUPLICATE KEY UPDATE `isbn`=VALUES(`isbn`)");
        } catch (\Exception $e) {
            print_r([$e->getMessage()]);
        }
    });
}

NOTE:

createOrUpdate function is only valid on model level and does not allow mass insert. Doing Model::insert() will allow to have a $data = [] inserted as long as fillable is used however it will reject if there is a duplicate key

implode works

        $insertString = '';
        foreach ($ch as $element) {
            $insertString .= '(' . $element . '), ';
        }

        $insertString = rtrim($insertString, ", ");

You can replace this section of code with just something like

        $insertString = '(' . implode('), (', $ch->all()) . ')';

This is shorter with fewer failure points.  And of course, it avoids having to trim off the extra comma at the end.

Bound parameters

But actually, it would be better to replace it with something like

        $insertString = implode(', ', array_fill(0, $ch->count(), '(?, ?)'));

and then pass the values to DB::insert as an array.  I'd write more of the code for this, but I'm not sure exactly how the Laravel Collection objects work.

Naming

Data is already plural (the singular is datum).  There is no datas.

I'd prefer $chunk to $ch.


Iterating through an array and staying within bounds Sorry if this question has been asked before, but I am having trouble searching for the answer I am looking for. This question is in regards to efficiency and overall C# standards.

I have an integer that I manipulate by 1, either by -- or ++ depending on a user's input. I then use this integer as an index to grab an object out of an array. I would like to know what the best way to make sure this variable will always stay within the confines of the array, such that if the user reached the end of the array, it would loop back to the beginning and continue from there. This is an example of the logic I am using for that:

switch(directionMoved)
{
    case MoveDirection.Left:
        currentPageIndex--;
        break;

    case MoveDirection.Right:
        currentPageIndex++;
        break;
}

if (currentPageIndex < 0)
{
    var distanceFromZero = currentPageIndex;
    currentPageIndex = objectArray.Length + distanceFromZero;
}

if (currentPageIndex >= objectArray.Length)
{
    var distanceFromEnd = currentPageIndex - objectArray.Length;
    currentPageIndex = distanceFromEnd;
}

...

(use 'currentPageIndex' to grab from array)

Is there a cleaner way to do this same logic? I hope my questions was clear... =]

Thanks!

You can use % remainder operator to be sure the value will be within bounds:

if (directionMoved == MoveDirection.Left) currentPageIndex--;
if (directionMoved == MoveDirection.Right) currentPageIndex++;

currentPageIndex += objectArray.Length;
currentPageIndex %= objectArray.Length;

This wiil allow you iterate in a circular way between 0 and objectArray.Length - 1.


Iterate through dynamic levels I have the below function that works and cycles through 3 possible levels. Is there a way to do the same as the function below but not have to do the multiple foreach statements, and not have to specify the number of levels as it is dynamic?

Basically the responses list can contain multiple GroupResponseTypes each of these can contain multiple ElementResponseBaseTypes which can be a variety of types themselves I'm interested in finding a value in an ElementResponseType And each ElementResponseBaseType itself can be a GroupResponseType, it to containing multiple types.

I'm looking at an easy way to scan through the whole structure for a specific Element.Reference and return the relevant value.

public static string GetValueFromFormField(List<ResponseBaseType> responses, string fieldref)
    {
        string fieldvalue = String.Empty;
        foreach (GroupResponseType groups in responses)
        {
            foreach (ElementResponseBaseType firstelements in groups.Responses)
            {
                if (firstelements.GetType() == typeof(ElementResponseType))
                {
                    if (firstelements.Element.Reference == fieldref)
                    {
                        ElementResponseType firstelement = new ElementResponseType();
                        firstelement = (ElementResponseType)firstelements;
                        fieldvalue = firstelement.Value;
                    }
                }
                else if (firstelements.GetType() == typeof(GroupResponseType))
                {
                    GroupResponseType secondgroup = new GroupResponseType();
                    secondgroup = (GroupResponseType)firstelements;
                    foreach (ElementResponseBaseType secondelements in secondgroup.Responses)
                    {
                        if (secondelements.GetType() == typeof(ElementResponseType))
                        {
                            if (secondelements.Element.Reference == fieldref)
                            {
                                ElementResponseType secondelement = new ElementResponseType();
                                secondelement = (ElementResponseType)secondelements;
                                fieldvalue = secondelement.Value;
                            }
                        }
                        else if (secondelements.GetType() == typeof(GroupResponseType))
                        {
                            GroupResponseType thirdgroup = new GroupResponseType();
                            thirdgroup = (GroupResponseType)secondelements;
                            foreach (ElementResponseBaseType thirdelements in thirdgroup.Responses)
                            {
                                if (thirdelements.GetType() == typeof(ElementResponseType))
                                {
                                    if (thirdelements.Element.Reference == fieldref)
                                    {
                                        ElementResponseType thirdelement = new ElementResponseType();
                                        thirdelement = (ElementResponseType)thirdelements;
                                        fieldvalue = thirdelement.Value;
                                    }
                                }
                            }
                        }
                    }
                }
            }
        }
        return fieldvalue;
    }

You are using Conditional, and i would recommend replacing these with Visitor.

See Replace Conditional with Visitor article by Martin Fowler.

The generic visitor pattern in the code snippet belo is useful for all kind of added operation types on the hierrarchy:

using System.Collections.Generic;
using System.Linq;
using NUnit.Framework;

namespace Traversal.Tests
{
    class VisitorTests
    {
        [Test]
        public void TestThis()
        {
            var topLevelResponse = new GroupResponse(
                new Response[]
                {
                    new ElementResponse("foo value", "foo"),
                    new GroupResponse(new[]
                        {
                            new ElementResponse("bar value", "bar")
                        })
                });

            var actualElement =
                topLevelResponse.Accept(new TraverseElementsVisitor()).FirstOrDefault(el => el.Reference == "bar");

            Assert.IsNotNull(actualElement);

            Assert.AreEqual("bar value", actualElement.Value);
        }
    }

    public interface IResponseVisitor<T>
    {
        T Visit(ElementResponse elementResponse);
        T Visit(GroupResponse groupResponse);
    }

    public abstract class Response
    {
        public abstract T Accept<T>(IResponseVisitor<T> visitor);
    }

    public class ElementResponse : Response
    {
        public ElementResponse(string value, string reference)
        {
            Reference = reference;
            Value = value;
        }

        public string Value { get; private set; }
        public string Reference { get; private set; }

        public override T Accept<T>(IResponseVisitor<T> visitor)
        {
            return visitor.Visit(this);
        }
    }

    public class GroupResponse : Response
    {
        public GroupResponse(IEnumerable<Response> responses)
        {
            Responses = responses;
        }

        public IEnumerable<Response> Responses { get; private set; }

        public override T Accept<T>(IResponseVisitor<T> visitor)
        {
            return visitor.Visit(this);
        }
    }

    public class TraverseElementsVisitor : IResponseVisitor<IEnumerable<ElementResponse>>
    {
        public IEnumerable<ElementResponse> Visit(ElementResponse elementResponse)
        {
            yield return elementResponse;
        }

        public IEnumerable<ElementResponse> Visit(GroupResponse groupResponse)
        {
            return groupResponse.Responses.SelectMany(r => r.Accept(this));
        }
    }
}

Your class hierarchy is not obvious to me, not sure i completely understand the class hierarchy you are using, so in the code example i provided a simplified one.

I'd say *Type postfix is redundant in this case, because you are using the objects to represent objects of a type, not the types themselves. Compare to usage of class name in System.Type, instances of which actually represent types.

Looks like you are putting same responsibility to multiple classes. Try to cleanly separate what is what, and what is common.


Improving time it takes to insert thousands of records using Entity Framework I have a task in my program that is inserting thousands (94,953 in one instance and 6,930 in another) of records into my database using Entity Framework.

Right now I am doing this and calling the .Add() method for each record but it takes about 1 minute to insert the smaller batch and over 20 minutes for the larger batch. I have tried the .AddRange() method but that jumped the smaller batch to over 4 minutes.

Is there another approach with Entity Framework 6 or is this a limitation I have to live with? If it makes any difference, the data is going into a SQL Server 2012 R2 instance.

        var taskCodes = DynamicsHelper.GetTaskCodes(dynamicsSession, "01-01-1990");
        Console.WriteLine("Adding task codes to database.");

        using (var db = new JobSightDbContext())
        {
            foreach (var taskCode in taskCodes)
            {
                var projectID = db.ProjectCodes.Where(project => project.Code == taskCode.Item4).Select(project => project.ID).FirstOrDefault();

                if (projectID != 0)
                {
                    var newTaskCode = new TaskCode()
                    {
                        Code = taskCode.Item1,
                        Description = taskCode.Item2,
                        IsActive = taskCode.Item3,
                        ProjectID = projectID
                    };

                    db.TaskCodes.Add(newTaskCode);
                    db.SaveChanges();
                }
            }

            Console.WriteLine("{0} tasks added to the database.", db.TaskCodes.Count());
        }

Since you're not making any changes to existing objects, you might set the AutoDetectChangesEnabledproperty of your context to false.

From MSDN:

  Gets or sets a value indicating whether the DetectChanges method is called automatically by methods of DbContext and related classes. The default value is true.

Calling the Add calls the DetectChanges method every time and is expensive. Turn it off and at the end, turn it on again.

Example:

using (var db = new DbContext)
{
    try
    {
        db.Configuration.AutoDetectChangesEnabled = false;
        //logic
    }
    finally
    {
        db.Configuration.AutoDetectChangesEnabled  = true;
    }
}

More reading on this: Secrets of DetectChanges Part 3

Also, you call the SaveChanges method every iteration in your loop. This means you make a call to your DB every iteration to persist that entity and is intensive and time consuming. Place that line of code outside your loop.

foreach (var taskCode in taskCodes)
{
    //logic
    db.TaskCodes.Add(newTaskCode);
}

db.SaveChanges();

It's much more performant to make 1 call for many items than many calls for 1 item!

Here's an example question on StackOverflow about this: Any difference between calling SaveChanges() inside and outside a foreach loop?


Making lines with SVG and JavaScript I just started trying out SVG the other day. Eventually I hope to be able to know how to do what SE does with their reputation graphs.

For now, I've just been trying to set up an easier way to make lines. I think my below code is decent, but please let me know what I can improve on or what I'm doing poorly.

Would like your thoughts before I start adding like animations, creativity, data to it.

Demo  http://jsfiddle.net/ECKNh/

var Line = {};

Line.LINES = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15];

var SVGline = function (l) {
    this.l = l;
}

for (var i = Line.LINES.length; i > -1; i -= 1) {
    Line[Line.LINES[i]] = new SVGline(Line.LINES[i]);
}

SVGline.prototype.createline = function (x1, y1, x2, y2, color, w) {
    var aLine = document.createElementNS('http://www.w3.org/2000/svg', 'line');
    aLine.setAttribute('x1', x1);
    aLine.setAttribute('y1', y1);
    aLine.setAttribute('x2', x2);
    aLine.setAttribute('y2', y2);
    aLine.setAttribute('stroke', color);
    aLine.setAttribute('stroke-width', w);
    return aLine;
}

function start() {
    var aSvg = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
    aSvg.setAttribute('width', 1000);
    aSvg.setAttribute('height', 400);
    var w = document.getElementById('container');

    for (var i = 1; i < 11; i += 1) {
        var x1 = Math.floor(Math.random() * 500 / 2);
        var xx = Line[i].createline(x1, 0, 200, 300, 'rgb(0,0,' + x1 + ')', i);
        aSvg.appendChild(xx);
    }
    w.appendChild(aSvg);
}

start();

Let me know what I can improve on or what I'm doing poorly.

If you are going to do animations with those lines, you will want to keep track of them. I am assuming that is why you have an array of LINES. However, in createLine you create the SVG element and just return it, you do not keep a reference to it. So it will be hard to animate those lines, you would have to query the DOM to find them back.

I suggest you have an object that creates SVG elements, a factory so to speak. And then another object that tracks lines you have created so far, mostly to do animations and other fun stuff.

The factory object could be something like

SVG = {
  createCanvas : function( width, height, containerId ){
    var container = document.getElementById( containerId );
    var canvas = document.createElementNS('http://www.w3.org/2000/svg', 'svg');
    canvas.setAttribute('width', width);
    canvas.setAttribute('height', height);
    container.appendChild( canvas );
    return canvas;
  },
  createLine : function (x1, y1, x2, y2, color, w) {
    var aLine = document.createElementNS('http://www.w3.org/2000/svg', 'line');
    aLine.setAttribute('x1', x1);
    aLine.setAttribute('y1', y1);
    aLine.setAttribute('x2', x2);
    aLine.setAttribute('y2', y2);
    aLine.setAttribute('stroke', color);
    aLine.setAttribute('stroke-width', w);
    return aLine;
  }
}

Initially the lines object could be as simple as

var lines = [];
lines.addLine = function( line ){
 this.push( line );
 return line;
}

Your start function would then be something like

function start() {
  var canvas = SVG.createCanvas( 1000 , 400 , 'container' ),
      lineElement, i, x1;

  for (i = 1; i < 11; i += 1) {
    x1 = Math.floor(Math.random() * 500 / 2),
    lineElement = lines.addLine( SVG.createLine(x1, 0, 200, 300, 'rgb(0,0,' + x1 + ')', i) );
    canvas.appendChild( lineElement );
  }
}


Simple C++ Hash function library I had created hash function library (MD5, MD4, SHA256, SHA384, SHA512, RipeMD128, RipeMD160, CRC16, CRC32, CRC64), written in C++.
Everything working well and My library produces exactly the same output compared to the PHP output. (Except for CRC series)
The individual algorithmic abstraction layers consist of the chash::IAlgorithm interface and chash::IDigest. But I'd like to refine IDigest more elegantly. How can I do it?
https://github.com/whoamiho1006/chash
IAlgorithm.hpp
#pragma once
#include "Macros.hpp"

namespace chash {
    enum class EAlgorithm {
        Unknown     = 0x0000,
        CRC16       = 0x1000,       // --> IBM Poly-Nomial.
        CRC32       = 0x1001,       // --> IEEE 802.3
        CRC64       = 0x1002,       // --> ISO Poly-Nomial.
        SHA256      = 0x2000,
        SHA384      = 0x2001,
        SHA512      = 0x2002,
        MD5         = 0x3000,
        MD4         = 0x3001,
        RipeMD128   = 0x4000,
        RipeMD160   = 0x4001,
    };

    enum class EAlgorithmErrno {
        Succeed = 0,
        InvalidState,
        InvalidDigest
    };

    class IDigest;
    class IAlgorithm {
    public:
        IAlgorithm(EAlgorithm type)
            : _type(type), _errno(EAlgorithmErrno::Succeed)
        {
        }

        virtual ~IAlgorithm() { }

    private:
        EAlgorithm _type;
        EAlgorithmErrno _errno;

    protected:
        inline void setError(EAlgorithmErrno _errno) {
            this->_errno = _errno;
        }

    public:
        /* get algorithm type. */
        inline EAlgorithm type() const { return _type; }

        /* get algorithm state. */
        inline EAlgorithmErrno error() const { return _errno; }

        /* create a new digest. */
        virtual IDigest* create() const = 0;

        /* initiate the algorithm. */
        virtual bool init() = 0;

        /* update the algorithm state by given bytes. */
        virtual bool update(const uint8_t* inBytes, size_t inSize) = 0;

        /* finalize the algorithm. */
        virtual bool finalize(IDigest* outDigest) = 0;

        /* compute hash with digest. */
        virtual EAlgorithmErrno compute(IDigest* outDigest, const uint8_t* inBytes, size_t inSize) {
            if (init()) {
                update(inBytes, inSize);
                finalize(outDigest);
                return error();
            }

            return error();
        }
    };

}

IDigest.hpp
#pragma once
#include "Macros.hpp"
#include <string>

namespace chash {
    class IDigest {
    public:
        virtual ~IDigest() { }

    public:
        /* get bytes pointer. */
        virtual uint8_t* bytes() const = 0;

        /* get size in bytes. */
        virtual size_t size() const = 0;

    public:
        inline std::string toHex() {
            std::string outHex;

            uint8_t* bytes = this->bytes();
            size_t size = this->size();

            outHex.reserve(size << 1);
            for (size_t i = 0; i < size; ++i) {
                int32_t b = bytes[i];

                int32_t fr = b / 16;
                int32_t bk = b % 16;

                if (fr < 10) outHex.push_back('0' + fr);
                else outHex.push_back('a' + (fr - 10));

                if (bk < 10) outHex.push_back('0' + bk);
                else outHex.push_back('a' + (bk - 10));
            }

            return outHex;
        }
    };

    /* Digest in template. */
    template<size_t Size>
    class TDigest : public IDigest {
    public:
        TDigest() {
            for (size_t i = 0; i < Size; ++i)
                _bytes[i] = 0;
        }

    private:
        mutable uint8_t _bytes[Size];

    public:
        /* get bytes pointer. */
        virtual uint8_t* bytes() const { return _bytes; }

        /* get size in bytes. */
        virtual size_t size() const { return Size;  }
    };
}

I have a follow up questions based on the answer by G. Sliepen.

Use the standard library

But I'd like to refine IDigest more elegantly. How can I do it?

Your IDigest basically reimplements std::vector<uint8_t>. Realizing this, I would therefore remove it entirely and replace it with std::vector<uint8_t>. You could add this to namespace chash if you like:
using Digest = std::vector<uint8_t>;

Your IDigest has limitations: you cannot compare two digests with each other. But you can with std::vector<uint8_t>. The only feature your class has is that it has a function to convert it to a string containing hexadecimal characters. I would leave that out entirely, and leave it up to the application to implement whatever they need to convert a digest to some other form, if they need that at all. But if you do want to provide this as a utility, simply create an out-of-class function to do this:
std::string toHex(const Digest &digest) {
    std::string outHex;
    outHex.reserve(digest.size() * 2);

    for (auto b: digest) {
        ...
    }

    return outHex;
}

Consider whether you need virtual base classes
Do you really need virtual base classes? These come with a cost: derived classes will now have a vtable, and function calls need to go through an extra level of indirection. Consider using a non-virtual base class, and make the base class's constructor protected, so you can no longer create an instance of a base class.


Simple form validation module Thinking about form validation, I just wanted something simple that works and is extensible.

I came up with the following code, which expects a list of selectors that maps to one or more functions performing validation. On success, a validator function returns a "falsey" value (empty string or false, preferably) and on success, it returns the error text. The final result is a map from all inputs to one or more errors they produced.

Any comments on style and good practices, and also good use of ES-6 functionality, are welcome.

export default function validateForm(form, rules, baseSelector = 'input,textarea') {
  let inputs = form.querySelectorAll(baseSelector);
  let errorMap = new Map();
  for (let input of inputs) {
    errorMap.set(input, validateInput(input, rules));
  }
  return errorMap;
}

function validateInput(input, rules) {
  let errors = [];
  for (let rule of Object.keys(rules)) {
    if (input.matches(rule)) {
      let e = validateRule(input, rules[rule]);
      errors.push(...e);
    }
  }
  return errors;
}

function validateRule(input, validators) {
  // In case only a single function is provided instead of an array
  if (typeof validators === 'function') {
    validators = [validators];
  }

  return validators
    .map(validate => validate(input))
    .filter(result => !!result);
}

Sample call:

function ensureRequired(input) {
  if (!input.required || input.value) {
    return '';
  }
  return 'required field not set';
}

function ensureNumber(input) {
  if (Number.isInteger(input.value)) {
    return '';
  }
  return 'field must be number';
}

let myForm = document.getElementById('myForm');
let validationResult = validateForm(myForm, {
  'input': ensureRequired,
  'input.phoneField': [ensureNumber]
};

Disclaimer: I'm aware HTML5 has built-in form validation - I was purposely trying to avoid using it.

Here are my thoughts:

Good use of arrow functions where appropriate. If you really wanted to, you could shorten some of those small functions- e.g.:

function ensureNumber(input) {
    if (Number.isInteger(input.value)) {
        return '';
    }
    return 'field must be number';
}

Could be shortened to a single line:

const ensureNumber = (input) => Number.isInteger(input.value)?'':'field must be number';

I also like the use of the spread operator when getting the errors back from the mapped validation for a field.
Depending on how the validationResult is used, a POJO could be used instead of a Map to utilize less memory, though that might not be a concern. A POJO doesn't have .forEach like Map would (though one could iterate over the keys of a POJO). Apparently when using an ES-2015 compliant engine, the key order is maintained.
Bearing in mind that it might not be preferable/possible to update the DOM, one could apply class names to the inputs/textareas/selects/etc. and then use Document.getElementsByClassName() instead of querySelectorAll() but that might only lead to a slight performance gain.


String.IndexOfAny(string, string[]) (lowest position for group of needles in haystack) This is a follow-up to this question with bug fixes, question and code improvements from @dfhwze, @PieterWitvoet, @HenrikHansen, @t3chb0t. I am still hoping for an improved approach or algorithm rather than micro-optimizations to the code.

.Net provides String.IndexOfAny(string, char[]) but not String.IndexOfAny(string, string[]).

The existing built-in String.IndexOfAny takes an array of char and returns the lowest position of any one char from the array in a passed in string or -1 if none are found.

My extension takes a string to search s and an array of strings to find targets and returns the lowest position found of any member of targets in s or -1 if none are found.

A naive implementation (using LINQ) isn't particularly efficient:

public static int IndexOfAny1(this string s, params string[] targets) =>
    targets.Select(t => s.IndexOf(t)).Where(p => p >= 0).DefaultIfEmpty(-1).Min();

My improved implementation tracks the current candidate position and restricts future searches to be before that candidate position:

public static int IndexOfAny2(this string s, params string[] targets) => s.IndexOfAny3(StringComparison.Ordinal, targets);
public static int IndexOfAny2(this string s, StringComparison sc, params string[] targets) {
    if (targets == null || targets.Length == 0)
        return -1;

    int index = s.IndexOf(targets[0], sc);
    var sLen = s.Length;

    for (int j1 = 1; j1 < targets.Length; ++j1) {
        var target = targets[j1];
        var targetIndex = s.IndexOf(target, 0, index >= 0 ? Math.Min(sLen, index + target.Length) : sLen, sc);

        if (targetIndex >= 0 && (index == -1 || targetIndex < index)) {
            index = targetIndex;
            if (index == 0) // once you're at the beginning, can't be any less
                break;
        }
    }

    return index;
}

This runs up to two times faster.

Sample code to test the two methods:

Console.WriteLine($"IndexOfAny1 should be 8: {"foo bar baz".IndexOfAny1("barz", "baz")}");
Console.WriteLine($"IndexOfAny1 should be 0: {"aabbccddeeffgghh".IndexOfAny1("bbb", "hh", "aa")}");
Console.WriteLine($"IndexOfAny1 should be 0: {"abc".IndexOfAny1("c", "abc")}");

Console.WriteLine($"IndexOfAny2 should be 8: {"foo bar baz".IndexOfAny2("barz", "baz")}");
Console.WriteLine($"IndexOfAny2 should be 0: {"aabbccddeeffgghh".IndexOfAny2("bbb", "hh", "aa")}");
Console.WriteLine($"IndexOfAny2 should be 0: {"abc".IndexOfAny2("c", "abc")}");

Is there a better algorithm or another way to make this faster?

PS Test harness for testing random possibilities:

var r = new Random();

var sb = new StringBuilder();
for (int j1 = 0; j1 < r.Next(80,160); ++j1)
    sb.Append((char)('0'+r.Next(0, 26+52)));
var s = sb.ToString();

var listTargets = new List<string>();
for (int j1 = 0; j1 < r.Next(5, 10); ++j1)
    if (r.NextDouble() < 0.8) {
        var tLen = r.Next(4, Math.Min(s.Length - 4, 10));
        var beginPos = r.Next(0, s.Length - tLen);
        listTargets.Add(s.Substring(beginPos, tLen));
    }
    else {
        sb.Clear();
        for (int j2 = 0; j2 < r.Next(5, 10); ++j2)
            sb.Append((char)('0'+r.Next(0, 26+52)));
        listTargets.Add(sb.ToString());
    }

var targets = listTargets.ToArray();
if (s.IndexOfAny1(targets) != s.IndexOfAny2(targets))
    Console.WriteLine($"Fail on {s} containing {String.Join(",", targets)}");

It seems that you've managed to fix the algorithm, so it does what it's supposed to do. But the concept is the same and performance isn't improved.

Still you could use some more descriptive names, and i instead of j1 (why 1?).

You could use foreach (string target in targets) { ... } instead of for (int j1;...) because you don't use the index to anything and the performance for an array is about the same for the two for-concepts.

You ask for other algorithm types that improves performance.

One concept for the algorithm - that seems to improve the performance significantly - could be the following:

public static int IndexOfAny(this string text, params string[] targets)
{
  if (string.IsNullOrEmpty(text)) return -1;
  if (targets == null || targets.Length == 0) return -1;

  for (int i = 0; i < text.Length; i++)
  {
    foreach (string target in targets)
    {
      if (i + target.Length <= text.Length && target == text.Substring(i, target.Length))
        return i;
    }
  }

  return -1;
}

Another that can improve performance even more is the following written in pseudo code - leaving it as a challenge for you to implement it in C#:

IndexOfAny text targets:
    for i = 0 to len(text)-1:
        skip = len(text)
        foreach target in targets:
            target_can_skip = 0
            for k = 0 to len(target)-1:
                if target[k] <> text[i+k]:
                    target_can_skip = count how many chars that can be skipped in text before target can be a candidate again
                    break
            if k == len(target):
                return i

            skip = min(skip, target_can_skip)

        if skip > 0:
            i = i + skip - 1

    return -1 // No match found

Besides that you may find inspiration here

Update according to VisualMelons comments:

The above implemented with string.IndexOf():

public static int IndexOfAny1(this string text, params string[] targets)
{
  if (string.IsNullOrEmpty(text)) return -1;
  if (targets == null || targets.Length == 0) return -1;

  for (int i = 0; i < text.Length; i++)
  {
    foreach (string target in targets)
    {
      if (i + target.Length <= text.Length && text.IndexOf(target, i, target.Length) == i)
        return i;
    }
  }

  return -1;
}

Notice that the count parameter must be at minimum the length of target or else it will not be found.

Test case:

  Stopwatch watch = Stopwatch.StartNew();
  IndexOfAnyDelegate[] funcs = new IndexOfAnyDelegate[]
  {
    Extensions.IndexOfAny,
    //Extensions.IndexOfAny1,
  };

    int sum = 0;

    for (int i = 0; i < 10000; i++)
    {
      foreach (var func in funcs)
      {
        sum += func("foo bar baz", "foo", "barz", "baz", " ");
        sum += func("aabbccddeeffgghh", "bbb", "hh", "aaa", "fg");
        sum += func("abcabc", "c", "abc");
        sum += func("abcabc", "x", "wer");
        sum += func("adfaææwjerqijaæsdklfjaeoirweqærqkljadfaewrwexwer", "xxxxx", "yyyyy", "zzzzz", "Aaaaaa", "x", "wer");

        //Console.WriteLine($"IndexOfAny should be 8: {func("foo bar baz", "barz", "baz", " ", "foo")}");
        //Console.WriteLine($"IndexOfAny should be 0: {func("aabbccddeeffgghh", "bbb", "hh", "aaa", "fg")}");
        //Console.WriteLine($"IndexOfAny should be 0: {func("abcabc", "c", "abc")}");
        //Console.WriteLine($"IndexOfAny should be 0: {func("abcabc", "x", "wer")}");
        //Console.WriteLine(func("adfaææwjerqijaæsdklfjaeoirweqærqkljadfaewrwexwer", "xxxxx", "yyyyy", "zzzzz", "Aaaaaa", "x", "wer"));
        //Console.WriteLine();
      }
    }
    watch.Stop();
    Console.WriteLine(sum);
    Console.WriteLine(watch.ElapsedMilliseconds);

You'll have to comment in/out as needed.

Update 2

The performance of the above pseudo code decreases (obviously) when the number of targets increases. So my analysis wasn't quite good enough. To optimize on that problem the below variant maintains an array of the next valid index per target, which minimize the number of targets that should be examined per char in the text string:

public static int IndexOfAny(this string text, params string[] targets)
{
  if (string.IsNullOrEmpty(text)) return -1;
  if (targets == null || targets.Length == 0) return -1;

  // Holds the next valid index in text per parget.
  int[] targetNextIndex = new int[targets.Length];

  for (int i = 0; i < text.Length; i++)
  {
    for (int j = 0; j < targets.Length; j++)
    {
      // If the targets next index isn't i then continue to next target
      if (targetNextIndex[j] > i)
        continue;

      string target = targets[j];
      int k = 0;

      for (; k < target.Length && i + k < text.Length; k++)
      {
        if (target[k] != text[i + k])
        {
          int nextIndex = i + 1;
          // Tries to find the next index in text where the char equals the first char in target.
          while (nextIndex < text.Length && target[0] != text[nextIndex])
          {
            nextIndex++;
          }
          // The next valid index for the target is found, so save it
          targetNextIndex[j] = nextIndex;
          break;
        }
      }

      if (k == target.Length)
      {
        return i;
      }
    }
  }

  return -1;
}


Cross-cutting delete flag in a repository I want to have a cross-cutting delete flag in my repository. I have the following repository pattern at the moment:

public interface IRepo<TModel> where TModel : EntityBase<int>
    {
        TModel Add(TModel item);
        void Add(IEnumerable<TModel> entities);
        TModel Remove(TModel item);
        TModel Find(int itemId);
        TModel Get(int itemId);
        IQueryable<TModel> Query { get; }
    }

I am exposing IQueryable which makes it hard to control whether deleted entities are retrieved or not in my service layer. Would adding a method to my repository which replaces Query and exposes a pre-filtered IEnumerable be bad form?

public IEnumerable<TEntity> GetAll
{
    get { return this.dbset.AsEnumerable().Where(x => x.IsDeleted == false); }
}

I am assuming since IQueryable is lazily loaded I won't pulling more entities into memory than required? Is this naive?

You don't need to call AsEnumerable on the DbSet in fact, doing so isn't great at all... As far as I am aware, this will make your Where clause execute in C# rather than being translated to SQL meaning a massive perf hit (I would be happy to be corrected here).

You should return IQueryable<TEntity> directly:

return dbSet.Where(x => !x.IsDeleted);

Notice that I've removed the redundant this. and also changed the comparison to use ! (not operator) instead of comparing to false.

One additional warning I would give you is: are you sure you need a generic base class repository? I find that when I have used this pattern a lot of my repositories get methods they never use.

It can also be argued that it's a leaky abstraction, see e.g here and here

I prefer slim interfaces of only a few methods rather than using header interfaces.


Simple string operations in C++ I'm a .NET (C#/VB) developer who knows a little C and happens to need to write a native C++ Windows DLL.

Please have a look at the following code and tell me if the string operations are idiomatic C++:

int dpi;

void __stdcall FooEntryPoint(ENTRY_INFO* inEntryInfo)
{
    dpi = *reinterpret_cast<int *>(inEntryInfo->UserData);

    OutputDebugStringW((std::wstring(L"User Data supplied: ") + std::to_wstring(dpi)).c_str());

    FARPROC address = GetFooAddress();
    if (address == NULL)
    {
        OutputDebugStringW(L"Foo address not found.\n");
        return;
    }

    NTSTATUS result = FrobnicateFoo(address);
    if (FAILED(result))
    {
        OutputDebugStringW((std::wstring(L"Failed to frobnicate foo: ") + RtlGetLastErrorString() + L"\n").c_str());
    }
}

My thoughts on the matter:

As a .NET developer, I'm used to "Unicode is default unless there's a very good reason against it", and, thus, I just use L"..." string literals and ...W API methods without giving it further thought.
(std::wstring(...) + ... + ...).c_str() seems to be the simplest way in C++ to do string concatenations of wchar arrays (denoted as ...) without having to worry about memory allocations. I could use wostringstream, but it would just make the code more complicated.
I know that failing silently is bad practice, but the signature of FooEntryPoint is part of the interface contract I'm given, so this is "by design".

Does that make sense or am I violating any good C++ practices here?

Note: The code does not need to be portable.

Some thoughts:

It is good that you explicitly annotated the function's definition with its required calling convention. Make sure that you do the same thing for the declaration in the public C++ header, and for the P/Invoke declaration (if applicable) in your .NET application.

Calling convention mismatches are a common source of bugs, and you don't want to rely on the correct compiler flags (specifying the default calling convention) having to be specified in order for the code to work correctly.
This line:

dpi = *reinterpret_cast<int *>(inEntryInfo->UserData);

hoists a giant red flag, for at least a couple of reasons:

Why is dpi a global (module-scoped?) variable? It looks to me like it just needs to be a local variable. I'm assuming that you raised its scope so it would be usable within the GetFooAddress and/or FrobnicateFoo functions? If so, you should pass it as a parameter to these functions. Always keep the scope of variables to the minimum level necessary—this is the same rule as in C#/VB.NET, of course.
That reinterpret_cast business is dicey. Reinterpretation casts are always a bit dicey, and this one especially, since it doesn't have a reassuring comment nearby that explains how it is safe. :-) You didn't show us the definition of the ENTRY_INFO type, so it's hard to confirm whether or not this actually is safe, but there really aren't that many cases where you can legitimately reinterpret a pointer to one primitive type as a pointer to another primitive type. If I had to guess, I'd say that UserData is a void*? If at all possible, you should just change it to be the int* that you ultimately want. The only advantage of a void* would be to achieve something akin to VB's Variant, but that requires some way to determine what the actual type is. Your code doesn't have that, it just assumes that the pointer is actually to an int. Lots of different ways that can go wrong. Hard to say exactly how you should fix this without more information, but definitely know that this smells funny and very likely hides a bug.
Beyond the question of the cast, you are potentially dereferencing a null pointer, which is undefined behavior and definitely a bug. You need to be checking this explicitly, just like you are all of the other error conditions, unless you can guarantee that the caller will never pass you a null pointer (in which case, verify that assumption using an assert).

  As a .NET developer, I'm used to "Unicode is default unless there's a very good reason against it", and, thus, I just use L"..." string literals and ...W API methods without giving it further thought.

This is completely correct. In fact, this doesn't have anything to do with .NET; it is more precisely a Windows thing. The Windows operating system uses Unicode internally (at least, all versions of NT do, which means everything modern and semi-modern), and there is no reason to ever use narrow/ANSI strings unless you need the code to run on Windows 9x. Therefore, your adopted practices make very good sense.

  (std::wstring(...) + ... + ...).c_str() seems to be the simplest way in C++ to do string concatenations of wchar arrays (denoted as ...) without having to worry about memory allocations.

Indeed, the + operator is the simplest way to concatenate strings in C++. However, it is not necessarily the most efficient. Use of the + operator is creating a bunch of temporaries, which the compiler may or may not be able to optimize out. In general, you should prefer the += operator to help stave off the proliferation of temporary objects (or the append member function, which is slightly less terse, but may be easier to reason about because there are no operator precedence concerns).

Also, not having to worry about memory allocations may be a curse disguised as a blessing. In general, pre-allocating the required amount of space (using the reserve member function) will net you better performance than letting the library code do incremental allocations as-needed, assuming of course that you have some idea in advance of how much space you will need.

That said, neither of these things matter if the code you're writing is not performance-sensitive. And obviously this isn't, since it's calling OutputDebugString, which is a very slow function.

  We're talking about a few bytes here. Isn't that a case of premature optimization? Or is it generally "good practice" in C++ to worry about such things?

Yes, it is probably "good practice" in C++ to worry about such things. A major reason for people to write code in C++ is because they care about efficiency.

  I could use wostringstream, but it would just make the code more complicated.

Well, that is arguable, and I see that πάντα ῥεῖ already made the argument. This is ultimately up to you: either will work, and since debug output is not performance-sensitive, there isn't a clear case to be made either way as far as I'm concerned. However, on the readability front, you can actually get a similar effect by using the += operator (which I'd recommended anyway—see code below).

I do want to quibble slightly with one thing πάντα ῥεῖ said, though—or at least, my interpretation of what he said… While it is, in general, completely true that more lines of code do not make a program slower or more complicated, using stream output probably is going to be slower than simple string concatenation. I would only resort to streams if I needed to format the output in some way.
Also, speaking of OutputDebugString, this is almost certainly a function that you only want to call in debugging builds. It won't be conditionally compiled out like the equivalent Debug functions in .NET; you have to do this manually. The simplest way is to use a preprocessor conditional, something like:

#ifdef DEBUG
    OutputDebugStringW(...)
#endif  // DEBUG

Another alternative would be to write your own TRACE macro (MFC and other commonly used C++ libraries have one included) that either expands to a call to OutputDebugString or to a no-op, depending on the build settings. For example:

#ifdef DEBUG
    #define TRACE(str)    OutputDebugStringW(str)
#else
    #define TRACE(str)    (void)(str)
#endif  // DEBUG

There are all sorts of ways to make this more complicated, more flexible, and more useful. In fact, you might not decide that you don't want to base it on the definition of a DEBUG preprocessor symbol at all, since you might want this debug trace available in an instrumented but otherwise optimized build. However, I would not recommend spewing a bunch of debug info in a shipping build. It is slow and unhelpful to the typical user.

If you really need this information for remote debugging, you should almost certainly be writing it into a log file, instead, since OutputDebugString is effectively an expensive no-op if there is no debug listener attached! Investigate a logging framework for C++, or write your own if you have unlimited time on your hands. Either way, make it so that this "verbose" mode can be enabled as a run-time option at the request of your technical support staff.
One last point about OutputDebugString: I personally believe that any string you pass to it should always end with a trailing new-line character. It doesn't get inserted automatically, so multiple back-to-back calls to OutputDebugString—even in completely different modules—will result in cluttered and unreadable output otherwise. A TRACE macro could certainly automatically append this \n, though.

  I know that failing silently is bad practice, but the signature of FooEntryPoint is part of the interface contract I'm given, so this is "by design".

Fair enough, but I have to again agree with πάντα ῥεῖ that you should be using an assert here for ease of debugging—unless that is, failure is an expected scenario.

You should be aware, though, that idiomatic C++ would use exceptions for things like this that cannot reasonably be handled by the callee, just like idiomatic C#/VB.NET code. This wouldn't affect the signature (the function would still return void), but it would certainly affect the interface, so I realize that may not be an option for you.
Instead of the NULL macro that you would use in C code, modern C++ code should prefer to use nullptr for a variety of reasons. You can basically use it as a drop-in replacement, with the advantage that it is a distinct type so it won't ever be ambiguous with the integer literal 0.
I also like to obsessively make every local variable that I'm not going to change in subsequent code const. Although this doesn't really enable any additional optimizations, it makes it easier for me to reason about the code and increases the likelihood that the compiler will catch bugs. In fact, I'm of the opinion that const should have been the default for locals, and you should have had to explicitly mark them as mutable if you wanted that. As a case in point, I would make the address and result variables explicitly const.
My last observation is almost purely stylistic, and probably at least somewhat controversial, but I would format the code differently to ensure only a single exit point. I'm not at all dogmatic about this, but I think in this particular case it is clearer to think about the operations you're doing in a chain or hierarchy, so it is natural to format them as a series of nested blocks:

const FARPROC address = GetFooAddress();
if (address != nullptr)
{
    const NTSTATUS result = FrobnicateFoo(address);
    if (FAILED(result))
    {
    #ifdef DEBUG
        std::wstring str = (L"Failed to frobnicate foo: ");
        str += RtlGetLastErrorString();
        str += L"\n";
        OutputDebugStringW(str.c_str());
    #endif  // DEBUG
    }
}
else
{
#ifdef DEBUG
    OutputDebugStringW(L"Foo address not found.\n");
#endif  // DEBUG
}

I'm neutral on whether the error condition (address == NULL) should come first, or whether the success condition should come first. There are readability arguments both ways. I tend to prefer putting the success condition first to increase the likelihood of successful static branch prediction, but that's becoming a non-issue on current generations of processors that don't simply assume "not taken" for all forward conditional jumps. I just wrote a crazy-long answer about this that ultimately concludes it doesn't matter.


Free memory outside function Is this going to leak memory or is it ok? Please also let me know if there is something else that I need to pay attention to.

typedef struct
{
int len;
UC * message;
}pack;

pack * prepare_packet_to_send(const int length,const unsigned char tag,const int numargs, ... )
{

pack *layer=  malloc(sizeof(pack));
va_list listp;
va_start( listp, numargs );
int step = 0;
layer->message = (unsigned char *) malloc(length);
layer->len = length;
int i = 0;
int len = 0;
unsigned char *source_message ;

for( i = 0 ; i < numargs; i++ )
{

source_message = va_arg( listp, unsigned char *);
len = va_arg( listp, long);
memcpy(layer->message+step, source_message, (long) len);
step+=len;
}

va_end( listp );
 return layer;
}

main()

{

pack *test = call prepare_packet_to_send(sizeof(var1)+sizeof(var2),any tag,any args)

// are following two frees correct/enough? or is there something else i need to do to     prevent mem leak?

free(test->message);
free(test);

}

Yes and no.

As you've used it here, I don't see a memory leak, so that would be a "no."

The function itself, however allocates memory without ever freeing it, so it's pretty much a memory leak waiting to happen -- the client code has to free the memory it allocates. Worse, freeing that memory is slightly non-trivial (two separate frees that have to happen in the right order) so it's not just a matter of whoever uses it remembering to free the memory, but there's also a possibility of them doing it wrong even if they do remember. Granted, it's not terribly complex either, but I'd still probably add a little function like:

void destroy_sent_packet(pack *p) {
    free(p->message);
    free(p);
}

So it's at least back to the client just remembering to free the memory.


Find out the 2 numbers which gives the highest product Input given is an int array, which may or may not contain positive, negative or zero values. Write a
  program to find out the two numbers which gives the highest product.
  (Both numbers should be positive or negative.)

Below is my code whose time complexity is O(n2), I believe. Is there any improvement I can make to the code?

public class FindLargestProduct{

    public static void main(String[] args) {
        int[] arr = { 52, 12, 34, 10, 6, 40, 0, 12, 40, 52, -56, -78, -99, 68 };
        int firstMax = 0, secondMax = 0;
        int firstNegMax = 0, secondNegMax = 0;

        for (int i = 0; i < arr.length; ++i) {

            if (arr[i] > 0) {
                if (arr[i] > firstMax) {
                    secondMax = firstMax;
                    firstMax = arr[i];
                } else if (arr[i] > secondMax) {
                    secondMax = arr[i];
                }
            }

            if (arr[i] < 0) {
                secondNegMax = firstNegMax;
                firstNegMax = arr[i];
                if (arr[i] > firstNegMax) {
                    secondNegMax = firstNegMax;
                    firstMax = arr[i];
                } else if (arr[i] > secondNegMax) {
                    secondNegMax = arr[i];
                }

            }
        }

        int proPositive = firstMax * secondMax;
        int proNegative = firstNegMax * secondNegMax;

        if (proPositive > proNegative) {
            System.out.println(proPositive);
        } else {
            System.out.println(proNegative);
        }
    }
}

Bugs

Your negative cases are wrong.

        if (arr[i] < 0) {
            secondNegMax = firstNegMax;
            firstNegMax = arr[i];

Here you always overwrite the current maximums...

            if (arr[i] > firstNegMax) {
                secondNegMax = firstNegMax;
                firstMax = arr[i];
            } else if (arr[i] > secondNegMax) {
                secondNegMax = arr[i];
            }

And here you check if a certain value is HIGHER than the max (which is negative, so 0 is higher than all negatives!). Neither of this is going to result in a good end result.

Aside from that, there seems to be no explicit check for verifying that you have at least an array of length 2 and that the output consists of either "two positive numbers" or "two negative numbers". Other than that, it's working fine.

Algorithm

I've got only one algorithmic improvement, and that's this one...

        if (arr[i] > 0) {
            if (arr[i] > firstMax) {
                secondMax = firstMax;
                firstMax = arr[i];
            } else if (arr[i] > secondMax) {
                secondMax = arr[i];
            }
        }

        if (arr[i] < 0) {
            secondNegMax = firstNegMax;
            firstNegMax = arr[i];
            if (arr[i] > firstNegMax) {
                secondNegMax = firstNegMax;
                firstMax = arr[i];
            } else if (arr[i] > secondNegMax) {
                secondNegMax = arr[i];
            }

        }

Here, you first check if the current element is positive... and then, later, you check if it's negative. Now, there's nothing wrong with that, but if you know that this element is positive, then there is no need for the negative check anymore. Use else-if here.

Naming

Your shorthand should be expanded, firstNegativeMax reads better than firstNegMax. max is well understood as "maximum", neg isn't as well understood. Aside from that, assigning arr[i] to a local variable with a name like "currentNumber" or "currentElement" would help readability. (Better yet, follow Martin R's advice and make it a for-each loop.)