The opposite of CodeLess Questions query What's the opposite of CodeLess Questions?
Perhaps CodeOnly Answers? (Also known as code dumps.)

Inspired by the opposite question, here's the opposite query:

SELECT Id AS [Post Link], Score, CreationDate
FROM Posts
WHERE PostTypeId = 2  -- answers
  AND Body LIKE '
%'
  AND CHARINDEX('
', Body) + LEN('
') = LEN(Body)
  AND ClosedDate IS NULL
ORDER BY CreationDate

Checking a couple of the results, this successfully finds the code only answers, and pretty fast.

I'm wondering if there's a better, cleaner approach that the hack with the CHARINDEX.

If your aim is to find code dumps, then you can do better.

Disclaimer: the example posts linked in the following paragraphs are code dump suspects. Maybe they are code dumps, maybe not. Judge for yourselves.

In particular,
answers that start with "I prefer:", or "Try:", or "Yes:",
and then followed by a code dump, won't be matched by the posted query,
but effectively they are still code dump suspects.

Similarly, answers that start with a code dump suspect and end with "etc...",
"and so on", or "?" are also code dump suspects.

Generally speaking, a code dump suspect looks like this:

Contains mostly code
Too short (or no) introductory text
Too short (or no) closing text

What is too short? There cannot be rule for that. The longer the text, the more false positives, and the shorter the text, the more code dumps missed. It's probably a good idea to make the text length before and after parameters of the query.

In addition to adding these parameters to widen the search,
it's natural to include the text before and after in the query result,
to make it easier to judge code dump suspects at a glance.

Something like this:

DECLARE @MaxTextLenBeforeCode AS INT = ##MaxTextLenBeforeCode:int?0##;
DECLARE @MaxTextLenAfterCode AS INT = ##MaxTextLenAfterCode:int?0##;

SELECT
  Id AS [Post Link],
  Score,
  CHARINDEX('
', Body) - 1 AS TextLenBeforeCode,
  LEFT(Body, CHARINDEX('
', Body) - 1) AS TextBeforeCode,
  LEN(Body) - CHARINDEX('
', Body) - LEN('
') AS TextLenAfterCode,
  RIGHT(Body, LEN(Body) - CHARINDEX('
', Body) - LEN('
')) AS TextAfterCode,
  CreationDate
FROM Posts
WHERE PostTypeId = 2  -- answers
  AND CHARINDEX('
', Body) > 0
  AND CHARINDEX('
', Body) - 1 <= @MaxTextLenBeforeCode
  AND CHARINDEX('
', Body) + LEN('
') >= LEN(Body) - @MaxTextLenAfterCode
  AND ClosedDate IS NULL
ORDER BY Score DESC, CreationDate


Simple calculator which can operate on more than two numbers All the calculator programs I have seen so far seem long, complicated and they can only operate on two numbers. Mine is simple, short, and it can operate on as many numbers as you want. Please tell me if there is another way to improve this section of code.

x=[]
amount=int(input("How many numbers?"))
operation=input("(*), (/), (+), (-)")
previous1 = 0
previous2=1
for i in range(amount):
    number=int(input("Number: "))
    x.append(number)
if operation == "+":
    for i in range(amount):
        previous1=x[i]+previous1
elif operation == "*":
    for i in range(amount):
        previous2=x[i]*previous2
elif operation == "-":
    for i in range(amount):
        previous1=x[i]-previous1
elif operation == "/":
    for i in range(amount):
        previous2=x[i]/previous2
if operation == "+" or operation == "-":
    print(previous1)
else:
    print(previous2)

reduce

Whenever you need to fold an operation across a list, like you're doing, you want to use reduce(). That takes a function, an iterable, and an optional initializer, and performs the operation across all of them. So:

reduce(f, [x, y, z])

is equivalent to:

f(f(x, y), z)

This is precisely what we want. The operator library additionally gives us all the mathematical operators, so we don't have to write them ourselves. So the various operations are:

+: reduce(operator.add, numbers) # or just sum(numbers)
-: reduce(operator.sub, numbers, 0)
*: reduce(operator.mul, numbers)
/: reduce(operator.floordiv, numbers, 1)

Simplifying the logic

We could directly translate the above into a dictionary of operators and a dictionary of initializers:

operators = {
    '+': operator.add,
    '-': operator.sub,
    '*': operator.mul,
    '/': operator.floordiv
}

initializers = {
    '-': 0,
    '/': 1
}

result = reduce(operators[operation],
    numbers,
    initializers.get(operation, None))

This will additionally fix your bug where for subtraction you're really adding.

List Comprehensions

It's more direct to simply initialize your numbers directly:

numbers = [int(input("Number: ")) for _ in range(amount)]

Note that x is not a good name. Prefer more descriptive names.

Division

All of your numbers are ints. Which means that when you divide, you're going to end up with 0 as soon as you have any number that isn't 1. That isn't particularly interesting, so I'd suggest using floating point numbers instead.


Passing a DTO into a static domain 'Create' method I'm wondering if it's bad practice to couple my DTO to my domain object like this and pass the object itself into Create(). Is it better to just give the parameters needed to perform the creation?

public static Playlist Create(PlaylistDto playlistDto, IUserManager userManager, IPlaylistManager playlistManager)
{
    Playlist playlist = new Playlist
        {
            Id = playlistDto.Id,
            Items = PlaylistItem.Create(playlistDto.Items, playlistManager),
            Sequence = playlistDto.Sequence,
            Title = playlistDto.Title,
            User = userManager.Get(playlistDto.UserId)
        };

    return playlist;
}

If the Playlist is an external dependency then it might make sense to use an extension method like:

public static Playlist Create(this PlaylistDto playlistDto, IUserManager userManager, IPlaylistManager playlistManager)
{
    Playlist playlist = new Playlist
        {
            Id = playlistDto.Id,
            Items = PlaylistItem.Create(playlistDto.Items, playlistManager),
            Sequence = playlistDto.Sequence,
            Title = playlistDto.Title,
            User = userManager.Get(playlistDto.UserId)
        };

    return playlist;
}

other than that, if the code is under your control, why not just make another constructor and do away with the static method


Rock, Paper Scissors game in Python I wrote a simple Rock, Paper Scissors game in Python for class. As a beginner, I would like some input as to how I could abstract or make my code smoother. I feel like the repeated use of if or elif statements are overbearing. I would like any tips or advice, or perhaps a different method of approach.

import random
user = ''

while user != 'q':
    print('Enter your choice')
    print('R or r for rock\nP or p for paper\nS or s for scissors\nQ or q to quit')
    user = input('')
    user = user.lower()
    if user == 'q':
        break
    comp = random.randrange(1, 4)
    choice = comp

    # assign (int) to user choices
    if user == 'r':
        user = 1
    elif user == 'p':
        user = 2
    elif user == 's':
        user = 3

    # assign string to computer random choice
    if   comp == 1:
        choice = 'rock'
    elif comp == 2:
        choice = 'paper'
    elif comp == 3:
        choice = 'scissors'

    print('Computer picked:', choice)

    # compare selection to figure out the winner
    if user == 1 and comp == 3:
        print('You win against the computer\n')
    elif user == 3 and comp == 1:
        print('You lose against the computer\n')
    elif user > comp:
        print('You win against the computer\n')
    elif user < comp:
        print('You lose against the computer\n')
    elif user == comp:
        print('You tie against the computer\n')

In Python you can compare multiple values the following way:

if (user, comp) in {(0, 2), (1, 0), (2, 1)} :
    print('You win against the computer\n')
elif user == comp :
    print('You tie against the computer\n')
else :
    print('You lose against the computer\n')


Removing punctuation and lowercasing a string I am very fresh to python. As part of an assignment, I've written the following code to remove punctuation from a string and convert it to lowercase.

import string

words = "Dave, Laura, Maddy, Dave, Laura, Maddy, Dave, Laura, Dave";
translation = str.maketrans("","", string.punctuation);
new = words.translate(translation);
lower = new.lower();

However, this seems crude to me and I feel like I can accomplish my task with fewer function calls/less code. Does anyone have any suggestions as to how I could do this?

Instead of just import string you could get the punctuation list directly:

from string import punctuation

words = "Dave, Laura, Maddy, Dave, Laura, Maddy, Dave, Laura, Dave"
translation = str.maketrans("", "", punctuation)

I'd also remove the semicolons. Python doesn't need them to know when a line of code ends since it uses newlines instead (there are exceptions, but none here) so you can remove them.


Clean up tags from a text I need to generate random text for tests. I do that using alphanumeric characters and some special characters as well.

For some special cases, I cannot generate the random text with tags.
I have the concept of a tag as text surrounded in curly braces (e.g. {name}).

I have written a simple code to clean up the tags (e.g. removing one of the curly braces) and I'd like some feedback if looks ok for Scala.

val allCharacters = "qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJKLZXCVBNM1234567890!@#%*()_+-=[]{x}|?.;:`"

def removeTags(text: String): String = {

    // A tag is any text between {}, like `{name}`
    def containsTag(text: String): Boolean = {
        val noTagsRegex: Pattern = Pattern.compile("^((?!(\\{.*\\})).)*$")
        !noTagsRegex.matcher(text).matches()
    }

    var newText = text
    if (containsTag(newText)) {
        val randomPosition = Random.nextInt(allCharacters.length - 1)
        newText = text.replace("}", allCharacters(randomPosition).toString)
    }
    newText
}

Below an idea of results running this method:

Text without tags

input: "dlfkhfks sldfjhsldf qiweo 93450934 %&%&%&^%"
output: "dlfkhfks sldfjhsldf qiweo 93450934 %&%&%&^%" (exactly the same)

Text with 1 tag

input: "dlfkhfks s{ldf}jhsldf qiweo 93450934 %&%&%&^%"
output: "dlfkhfks s4ldf}jhsldf qiweo 93450934 %&%&%&^%" (the { was replaced)

Text with more than 1 tag

input: "dlfkhfks s{ldf}jhsldf {{@}} qiweo 93450934 %&%&%&^%"
output: "dlfkhfks s4ldf}jhsldf 44@}} qiweo 93450934 %&%&%&^%"

There is room for improvements, like not doing replace to all occurrences by the same character, but this is something for another moment.

Beautify your regex
Say No to Pattern.compile and instead of

 Pattern.compile("^((?!(\\{.*\\})).)*$")

use scalish way:
 "^((?!(\\{.*\\})).)*$".r

Say No to \\ in regex and instead of

"^((?!(\\{.*\\})).)*$"

use """ way:
"""^((?!(\{.*\})).)*$"""

Return values without val/var
You can simply write your containsTag without val and result of last line will return from function - it will return your's matches:
def containsTag(text: String): Boolean = {
  ! """^((?!(\{.*\})).)*$""".r.matches(text)
}

Choose your destiny
Sometimes it is good to use if/else but more readable match/case:
containsTag(text) match {
  case true =>
    val randomPosition = Random.nextInt(allCharacters.length - 1)
    text.replace("}", allCharacters(randomPosition).toString)
  case _ =>
    text
}

Feel the power
Collect it all together:
  val allCharacters = "qwertyuiopasdfghjklzxcvbnmQWERTY" +
    "UIOPASDFGHJKLZXCVBNM1234567890!@#%*()_+-=[]{x}|?.;:`"

  def removeTags(text: String): String = {

    def containsTag(text: String): Boolean = {
      ! """^((?!(\{.*\})).)*$""".r.matches(text)
    }

    containsTag(text) match {
      case true =>
        val randomPosition = Random.nextInt(allCharacters.length - 1)
        text.replace("}", allCharacters(randomPosition).toString)
      case _ =>
        text
    }
  }

  val res = removeTags("my friendly {name} is $userNickname$")

For random chars
According to answer in Replacing characters in a String in Scala you can use construction without while loop but with map + lambda like in code below:
  val allCharacters = "+=-?"

  def removeTags(text: String): String = {

    def containsTag(text: String): Boolean = {
      ! """^((?!(\{.*\})).)*$""".r.matches(text)
      true
    }

    containsTag(text) match {
      case true =>
        text.map(
          (c) => c match {
            case '{' =>
              val randomPosition = Random.nextInt(allCharacters.length - 1)
              allCharacters(randomPosition)
            case c => c // for another char which is not == {
          }
        )
      case _ =>
        text
    }
  }

Then it will be good to divide functionality and create new Char => Char transformer function from anonymous (c) => c match {..} which we lay in text.map(...):
//...
def replaceRandom(c: Char): Char = {
  c match {
    case '{' =>
      val randomPosition = Random.nextInt(allCharacters.length - 1)
      allCharacters(randomPosition)
    case c => c
  }
}

containsTag(text) match {
  case true => text.map((c) => {replaceRandom(c)})
  case _ => text
}
//...

Simplify code in two parts.
Part 1: simplify match/case in replaceRandom(c: Char): Char :
def replaceRandom(c: Char): Char = {
  c match {
    case '{' =>
      val randomPosition = Random.nextInt(allCharacters.length - 1)
      allCharacters(randomPosition)
    case c => c
  }
}

// remove unneeded "{" and "}"
def replaceRandom2(c: Char): Char = c match {
  case '{' =>
    val randomPosition = Random.nextInt(allCharacters.length - 1)
    allCharacters(randomPosition)
  case c => c
}

Then part 2: simplify map((c) => {replaceRandom(c)}):
containsTag(text) match {
  case true => text.map((c) => {replaceRandom(c)})
  case _ => text
}

//remove unneeded "{" and "}"
containsTag(text) match {
  case true => text.map((c) => replaceRandom(c))
  case _ => text
}

//remove unneeded "(c)" to wildcard "_"
containsTag(text) match {
  case true => text.map(replaceRandom(_))
  case _ => text
}

//remove unneeded wildcard "_"
containsTag(text) match {
  case true => text.map(replaceRandom)
  case _ => text
}

Feel the power again
So for your task i will chose that variant: all functions are divided, logic placed is simple if/else and now good readable (as you see now it good readable also with if/else as i wrote to you above). After, when logic will grow you can use instead if/else match/case.
  def removeTags(text: String): String = {

    def containsTag(text: String): Boolean = {
      ! """^((?!(\{.*\})).)*$""".r.matches(text)
      true
    }

    def replaceRandom(c: Char): Char = c match {
      case '{' =>
        val randomPosition = Random.nextInt(allCharacters.length - 1)
        allCharacters(randomPosition)
      case c => c
    }

    if (containsTag(text))
      text.map(replaceRandom)
    else text

  }

  val allCharacters = "+=-?"
  val e = "my fr{{{iend{{{ly {{name} is $userNickname$"
  println(removeTags(e))

P.S. or place to think

Maybe your regex not working? Check it at: Regex101
Why you need to replace } symbol with another random from allCharacters  which can be also }?
The bad, the good and the !: maybe try to rewrite your !noTagsRegex to noTagsRegex? It can be more readable for another people.

Also i want to see additions to my review too, that code is too verbose, so welcome!


Test whether a string representing an object n-levels deep is available I was writing some JavaScript that depended on several global variables being present.

I read about the "guard pattern" from a StackOverflow answer but since I need to check for multiple objects it quickly becomes quite tedious to have lines like this everywhere:

if (foo && foo.bar && foo.bar.baz) {
    // access foo.bar.baz
}

I wanted to replace the above with:

if (isAvailable("foo.bar.baz")) {
    // access foo.bar.baz
}

Here is the implementation for isAvailable:

// test whether a string that represents an object in a certain
// context (using dot notation) of n-levels deep is available
function isAvailable(s,context) {
    var aSplit, ob;

    if (s === undefined || s === null) {
        return false;
    }

    aSplit = s.split('.');
    ob = context[aSplit.shift()];
    while (ob !== undefined && aSplit.length > 0) {
        ob = ob[aSplit.shift()];
    }
    return typeof ob === "object";
}

Here is a JSBin: http://jsbin.com/jokowomepo/edit?js,console

Also, I should mention that I am only interested in solving this problem with vanilla JavaScript...no libs.

My implementation seems to work, but I'd be interested if there are improvements that could be made.

Any feedback?

This maybe a matter of taste, but I'm not a big fan of slicing arrays.
I find this alternative implementation using a for loop and without array slicing a bit simpler:

function isAvailable(s,context) {
    var aSplit, ob, i;

    if (s === undefined || s === null) {
        return false;
    }

    aSplit = s.split('.');
    ob = context;
    for (i = 0; ob !== undefined && i < aSplit.length; ++i) {
        ob = ob[aSplit[i]];
    }
    return typeof ob === "object";
}


Implementation of memmove Here is my implementation of memmove; where can I improve?

void* memmove(void* dest, const void* src, std::size_t count)
{
    char *dest_ = static_cast<char*>(dest), *src_ = (char*)(src);
    if ((char*)src + count > dest && src < dest) //
    {
        dest_ += (src_ += count - 1, count - 1);
        while (count--) *dest_-- = *src_--;
    }
    else while (count--) *dest_++ = *src_++;
    return dest;
}

Missing header: #include <cstddef> is required for std::size_t (other headers also provide it).
There's no need for src_ to cast away the constness of *src:
char *dest_ = static_cast<char*>(dest);
char const *src_ = static_cast<char const*>(src);

Personally, I'd just go for d and s rather than the ugly trailing underscores, but that's very much a matter of taste.
Arithmetic and comparison between pointers to different objects is unspecified behaviour, and I don't see a way to avoid it in the test.  Thankfully, in the case where src and dest are in different objects, it doesn't matter which branch you take
There's no need to test for src + count > dest as well as src < dest; we can quite happily use the "reverse" copy for any lower-to-higher copy, as long as we don't subtract from count==0 in that branch.
In the "reverse" branch, it's disingenuous to hide the assignment to src_ inside the assignment to dest_.  Much clearer to write as:
    src_ += count - 1;
    dest_ += count - 1;

We can avoid that -1 if we pre-decrement rather than post-decrement (it's safe to point to one past the end).
Style: Use braces on both sides of else, or neither; avoid if () {} else ;.

My version
#include <cstddef>

void *memmove(void *dest, const void *src, std::size_t count)
{
    auto d = static_cast<char*>(dest);
    auto s = static_cast<char const*>(src);

    // If s and d are in distinct objects, the comparison is
    // unspecified behaviour, but either branch will work.
    if (s < d) {
        s += count;
        d += count;
        while (count--)
            *--d = *--s;
    } else {
        while (count--)
            *d++ = *s++;
    }

    return dest;
}

Performance
Copying a single char at a time is going to be sloowww.  Real implementations take advantage of processors' native transfer size, DMA hardware, and specialized instructions where available.  That's why it's a standard library function rather than user-supplied.


jsperf for js objects vs array tuples I'm getting more and more into programming js in a functional manner which makes heavy use of multiple returns via tuples (especially useful with recursion). Also, a pattern that I use often is creating simple temporary values for projections

_.chain(arr)
       .filter(...)
       .map(function(x){ return [x.foo, getSomething(x.bar)] })
       .filter(...)
       .reduce(...)

Usually I see tuples in js implemented with arrays [a, b]  though of course an object - while annoying to write would be just as valid: {a: a, b: b}.

So for this sort of use-case (creation and light usage of a bunch of temporary objects) I'm curious if there's a significant performance difference so I set up this jsperf.

Object tuple test:

var all = []
for(var x = 0; x < 100; x+=1)
for(var y = 0; y < 100; y+=1)
  all.push({x: x, y: y})

var mapped = []
for(var i=0; i < all.length; i+=1)
  mapped.push({x: all[i].x, y: all[i].y})

versus array tuple test

var all = []
for(var x = 0; x < 100; x+=1)
for(var y = 0; y < 100; y+=1)
  all.push([x, y])

var mapped = []
for(var i=0; i < all.length; i+=1)
  mapped.push([all[i][0], all[i][1]])

What do people think? Is this a sufficient test for the concept?

You should consider testing only the creation of the object/array, i.e. [ x, y ] vs. { x: x, y: y }, and also how it reacts to more items:

http://jsperf.com/array-vs-object-tuple

I'd say that the conclusion would be that there is no dramatic performance difference, and they both react reasonably to an increased number of items. Either is cheap enough that the actual performance of the code is usually decided by the actual work of getting the values to put in the object/array.

So, you should normally use the one that makes for conventient use of the result, and maintainable code. Having the ability to name the items in the result can definitely be convenient if you have more than just a few.


Algorithm to rotate array elements in Ruby I wrote this algorithm to rotate elements in an array. It's not very efficient, but it works. It has another disadvantage that it doesn't rotate right (it would be nice to pass it negative steps, for example).

def rotate_left!(array, steps = 1)
  for i in 0...steps
    first = array[0]
    for j in 1...array.size
      array[j - 1] = array[j]
    end
    array[-1] = first
  end
end

An aside, does this count as \$O(n^2)\$? For each step, we have to iterate over the entire array and shift positions by -1.

Let \$A\$ be the size of the array, \$S\$ be the steps, and \$C\$ be the constant operations of storing the first element then adding it to the end of the array.

$$
T(A, S) = S(A) + S(2C)
$$

This notation is ad-hoc, so excuse me if it doesn't make sense.

I am aware of array#rotate, and of array#shift and array#pop. For example:

def my_rotate!(array, steps = 1)
  for i in 1..steps
    array.push(array.shift)
  end
end
# or
array.push(array.shift(steps)).flatten!

But I'm curious if there are other ways that rely less on Array instance methods.

Your algorithm is \$O(N*M)\$ where:
\$N\$ = size of array
\$M\$ = steps

It's possible to do an array rotation that's \$O(N)\$, regardless of the number of steps you specify though.

Start by thinking of the array as two pieces, separated at "steps" indices into the array. For example, if you're going to rotate by 4, then mentally separate it into array[1..5] and array[5..N].

To do the rotation, start by reversing each of those pieces. Then reverse the entire array, something on this order:

for j in 1...steps/2
    temp = array[j]
    array[j] = array[steps-j+1]
    array[steps-j+1] = temp
end

for k in 1...(array.size-steps)/2
    temp = array[steps+k]
    array[steps+k] = array[array.size-k+1]
    array[array.size-k+1]=temp
end

for i in 1...array.size
    temp = array[i]
    array[i] = array[array.size-i+1]
    array[array.size-i+1] = temp
end

Note: I'm more accustomed to C-style arrays, where indexes start at 0 instead of 1. I've tried to compensate appropriately, but I haven't tested this, so I wouldn't be surprised if there were still a few off-by-one errors.

This steps through the entire array twice, regardless of the size of rotation, so it's likely to be slower than yours for steps=1, about the same speed for steps = 2, and faster for steps > 2.


Calculate Mersenne prime numbers in MATLAB A Mersenne prime is equal to (2^n - 1), where n is an integer.
31 is a Mersenne prime since 31 = 2^5 -1. I need to write a code that calculates Mersenne primes between 1 and 10,000. Lastly I'm not allowed to use the build in function isprime.

I have used the formula to calculate till an arbitrary n then used a for loop to verify if the numbers I got are prime numbers.

Running the script I get  3, 7, 31, 127, 8191.

How could I make the code look cleaner?
Do you think my code is giving me the right answer?

    for k = 1:50000 %calculates mersenne primes using mersenne formula

        p(k) = 2^k -1;

    end
    p = p(p<10000 & p>1);
    psize =size(p);
    x = psize(2);
 mersenneprime = 0; k = 2; f = 0;

for m = 1:x %verifies if mersenne are prime numbers
    k = k +1;
a = p(m);
b = p(m)-1;
    if all((rem(a,2:b)) ~= 0)
          flag = 1;
      else
          flag = 0;
    end
      if flag == 1
          f = f+1;
          mersenneprime(f) = p(m);
      end
end %it eliminates 7 numbers from original array
mersenneprime

I can comment on multiple things here. But you ask about making the code look cleaner, so I'll start with that

Indenting

Be consistent with indenting. Indents show the structure of your code. Inconsistent indenting makes it hard to follow the structure.

Consistent spacing around operators also helps readability.

Variable names

It's common in MATLAB code to use single-letter variable names. I do this too. Try to use variable names that make the purpose of the variable clear. For example, p could be called candidates, since they are the possible Mersenne prime candidates.

Variable usage

psize = size(p);
x = psize(2);
% <snip>
for m = 1:x

You define a variable x here, but then use it only once. The name x is meaningless, so it takes a bit of time to find out what you are looping over. Consider instead:

for m = 1:length(p)

That one statement is immediately clear.

But since you are only using m to index into p (a = p(m)), you can also do this:

for a = p

Now m will have one of the values of p in each loop iteration (you don't need to do the indexing). Note that this form of for iterates over the columns of p, so if p were a column vector instead of a row vector, then the loop would only run once, with a containing all values in p.

You actually index p(m) three times within this loop. Two of those you can replace with a, which you have assigned the value of p(m).

Finally, k is initialized, and incremented each loop iteration, but not used at all. Unnecessary assignments and operations add confusion to your code, try to avoid these.

Vectorize

There are several places where you can simplify your code.

for k = 1:50000 %calculates mersenne primes using mersenne formula
   p(k) = 2^k -1;
end

This is identical to

p = 2.^(1:50000) - 1;

(note the .^ element-wise operator). Here you generate 50 thousand powers of two, but next you keep only 12:

p = p(p<10000 & p>1);

Why not directly generate the values you intend to keep? For the condition 2.^k-1 < 10000, you need k < log2(10000-1), and for 2.^k-1 > 1 you need k > 1:

p = 2.^(2:log2(10000-1))-1;

Testing for primeness

Next, you loop over each of the candidates to see if it's a prime:

a = p(m);
b = p(m)-1;
if all((rem(a,2:b)) ~= 0)

Note that to test for primeness, you need to test division only up to sqrt(a), any larger value is guaranteed not to divide the value evenly. And you don't need to test with any power of 2 except 2 itself:

if all(rem(a,[2,3:2:sqrt(a)]) ~= 0)

You could consider making this test into a function, which will improve readability (then you don't need to add the comment):

isprime = @(a)all(rem(a,[2,3:2:sqrt(a)]) ~= 0);
% ...
if isprime(a)

(now you're using the isprime function, but not the built-in one!)

Simplify logic

Within this test you set flag = 1 or flag = 0. Considering true has a value of 1, and false a value of 0, you might as well do:

flag = isprime(a);

But next you test flag and add the number to the list of Mersenne primes. So you don't need the flag at all:

if isprime(a)
   f = f+1;
   mersenneprime(f) = a;
end

Appending to an array

Adding a value to the mersenneprime list can be simplified too, using the end keyword in indexing, do you don't need f:

mersenneprime(end+1) = a;

This is actually fairly efficient. They have tried to make people preallocate arrays like these that grow within a loop for decades, and finally decided to optimize their allocation strategy for this use case. Also, since your list will always be very small, no matter how large your upper limit is, you don't need to worry about efficiency here at all. You do need to initialize the mersenneprime to an empty array.

Putting it all together

So putting this all together I get to the following bit of code:

isprime = @(a)all(rem(a,[2,3:2:sqrt(a)]) ~= 0);
limit = 10000;
candidates = 2.^(2:log2(limit-1))-1;
mersenneprime = [];
for c = candidates
   if isprime(c)
      mersenneprime(end+1) = c;
   end
end
disp(mersenneprime)


Exposing includes to client over webservice for database queries I am using a CQRS type patter for querying data in my application. I am currently using entity framework to connect to the database.

public class GetCustomerQuery : IQuery<Customer>
{
     public string CustomerId { get; set; }
}

public class GetCustomerQueryHandler : IQueryHandler<GetCustomerQuery, Customer>
{
    public void Handle(GetCustomerQuery query)
    {
        return _dbContext.Customers.FirstOrDefault(c => c.Id == query.CustomerId);
    }
}

The problem I am having with this design is that the Customer object has relationships with other entities:

public void Customer
{
    public string Id { get; set; }
    public IList<Post> Posts { get; set; }
}

The query handler shown above will not return the list of posts, as this is a one-to-many relationship in the database. The way I was thinking to solve this was to add a BaseQuery object which allowed settings includes:

public abstract class BaseQuery<T> : IQuery<T>
{
    public void AddInclude(Expression<Func<T, object>> include)
    {
      ....
    }
}

Is this considered a leaky abstraction, as it exposes the structure of the database?

Is there a better way around this problem?

I don't think you should add a method for setting includes on the query object. That means your consumers have to know which properties need to be included and which ones don't.

I think you should be explicit about it on the query. One approach could be to simply add a property:

// Slightly odd use of "WithPosts" to make it more comparable.
public class GetCustomerQuery : IQuery<Customer>
{
    public string CustomerId { get; private set; }

    public bool IncludePosts { get; private set; }

    public GetCustomerQuery(string customerId)
    {
        CustomerId = customerId;
    }

    public GetCustomerQuery WithPosts()
    {
        return new GetCustomerQuery(CustomerId) { IncludePosts = true};
    }
}

You could also leverage some inheritance:

public class GetCustomerWithPostsQuery : GetCustomerQuery
{
    public GetCustomerWithPostsQuery(string customerId) : base(customerId)
    {
    }
}

public class GetCustomerQuery : IQuery<Customer>
{
    public string CustomerId { get; private set; }

    public GetCustomerQuery(string customerId)
    {
        CustomerId = customerId;
    }

    public GetCustomerWithPostsQuery WithPosts()
    {
        return new GetCustomerWithPostsQuery(CustomerId);
    }
}

Either way:

var customerQueryWithoutPosts = new CustomerQuery("Dennis");
var customerQueryIncludingPosts = new CustomerQuery("Dennis").WithPosts();
// ... profit?

You can then either check the query's IncludePosts property OR have a different handler for the specialised GetCustomerWithPostsQuery query object.


Element inside a website I'm finally "done" with my small prototype for an element inside my website. I'm just a learner in the jQuery code and that reflects in my code. It's bloated, a lot of functions do the same thing, but for a red or green button which results in this kind of code.

$('.submenu').on('click','.rood',function() {
    $(this).closest('.container_vragen').fadeOut(400, function() {
        $(this).closest('.container_vragen').css({overflow : 'hidden', color : 'red'});
        $(this).closest('.container_vragen').appendTo("#niet_geregeld");
        $(this).closest('.container_vragen').fadeIn(400);
    });
});

$('.submenu').on('click','.groen',function() {
    $(this).closest('.container_vragen').fadeOut(400, function() {
        $(this).closest('.container_vragen').css({overflow : 'hidden', color : 'green'});
        $(this).closest('.container_vragen').appendTo("#geregeld");
        $(this).closest('.container_vragen').fadeIn(400);
    });
});

As you can see most of the functions are the same, only the color and the .appedTo id changes. What's the best way to merge this code? I'm thinking there needs to be an if statement which filters what button is clicked. But I can't figure it out on my own.

Make a function.

function doStuff(obj, c) {
  obj.css({overflow: 'hidden', color: c}).fadeIn(400);
}

Then call it

$('#submenu').click(function () {
  doStuff($('#geregeld'), 'green');
});

(simplified example)

Feel free to mess around: http://jsfiddle.net/rQFHt/


Dynamically add/remove fields on input The following code allows users to enter multiple optional quantity fields after first filling in a value of the current quantity input. If the input field contains a value it will then display the next quantity field, if not, it will remove/not display it.

The code is currently working as intended but I don't believe that it follows the DRY principle very well. Is there a way to make this more succinct?

function quantityCheck() {
    var q1 = document.getElementById("Quantity1Value").value;
    if (q1 !== "") {
        document.getElementById("Quantity2").style.display = "";
    } else {
        document.getElementById("Quantity2").style.display = "none";
    }

    var q2 = document.getElementById("Quantity2Value").value;
    if (q2 !== "") {
        document.getElementById("Quantity3").style.display = "";
    } else {
        document.getElementById("Quantity3").style.display = "none";
    }

    var q3 = document.getElementById("Quantity3Value").value;
    if (q3 !== "") {
        document.getElementById("Quantity4").style.display = "";
    } else {
        document.getElementById("Quantity4").style.display = "none";
    }
}

    Quantity 1
    <input type="number" id="Quantity1Value" oninput="quantityCheck()" required>

    Quantity 2
    <input type="number" id="Quantity2Value" oninput="quantityCheck()">

    Quantity 3
    <input type="number" id="Quantity3Value" oninput="quantityCheck()">

    Quantity 4
    <input type="number" id="Quantity4Value">

It would be better to do your evaluation only for the particular elements that could be affected each time rather than to reevaluate all of them on any change in any input.

We can rewrite this to pass along the element that triggered the event and the ID of the element that we want to update the style on. Take a look at this:

function quantityCheck(eventElement, quantity) {
 //try to use const or let instead of var.
 //by passing 'this', we now have the target element that triggered the event,
 //so we only need to grab the one we want to change.
 const quantToChange = document.getElementById(quantity);

 //we can also use a tertiary function to make our change to shorten things a bit.
 quantToChange.style.display = eventElement.value.length ? '' : 'none';
}

    Quantity 1
    <input type="number" id="Quantity1Value" oninput="quantityCheck(this, 'Quantity2')" required>

    Quantity 2
    <input type="number" id="Quantity2Value" oninput="quantityCheck(this, 'Quantity3')">

    Quantity 3
    <input type="number" id="Quantity3Value" oninput="quantityCheck(this, 'Quantity4')">

    Quantity 4
    <input type="number" id="Quantity4Value">


One-dimensional circular Conway's Game of Life I have a program that is working, but it takes way too long to get the answer. With input like

15 100000000
111010111001100

The program checks for 15 numbers, and checks if two number beside the numbers in the second row adds up to exactly 1. If it adds up to 1, then the number would stay/change to 1, otherwise it change/stay 0. And this need to happen for 100000000 times (it changes each time). Right now I can get the right answer, but it takes like 1 minute to get it.

import java.util.*;
public class circle {
final Scanner in;
public circle()
{
   in=new Scanner(System.in);
   run();
   in.close();
}
public void run()
{
   String input=in.nextLine();
   //I created two arrays, checks for one array while change the other one so the changed number wouldn't affect other checking
   final int[] list=createarray(input);
   final int totalcells=list[0];
   final int generation=list[1];
   String input1=in.nextLine();
   long[] deadalive=createarray2(input1,totalcells);
   long[] changing=new long[totalcells];
   System.arraycopy(deadalive,0,changing,0,totalcells);
   //It checks for y times and checks all the cells
   for(int y=0;y<generation;y++)
   {
       for(int x=0;x<totalcells;x++)
       {
           if(x==0)
           {
               if(deadalive[x+1]+deadalive[totalcells-1]==1)
               {
                changing[x]=1;
               }
               else
                   changing[x]=0;

           }
           else if(x==totalcells-1)
           {

                   if(deadalive[x-1]+deadalive[0]==1)
                   {
                       changing[x]=1;
                   }
                   else
                       changing[x]=0;
           }
           else
           {
                   if(deadalive[x-1]+deadalive[x+1]==1)
                   {
                       changing[x]=1;
                   }
                   else
                       changing[x]=0;
           }
       }
       //make two arrays the same after each changes
       System. arraycopy(changing, 0, dead alive, 0, changing.length);

   }
   System.out.println(java.util.Arrays.toString(changing));

}
private static int[] createarray(String input)
{
   final String[] args=input.trim().split("\\s+");
    final int aray[]=new int[2];
    for(int i=0;i<2;i++)
    {
        aray[i]=Integer.parseInt(args[i]);
    }
    return aray;
}
private static long[] createarray2(String input1,int totalcells)
{
   final long array[]=new long[totalcells];
    for(int i=0;i<input1.length();i++)
    {
        array[i]=Character.digit(input1. charAt(i),10);
    }
    return array;
}
public static void main(String args[])
{
   new circle();
}
}

This code is supposed to change the second row of numbers like input:

7 1
0000001

Output would be:

1000010

0 changes to 1 if the two "neighbor" adds up to 1 exactly. other wise they are 0, but this one only runs for one time unit, where there are ones that needs to run 10000000 time units. For loop probably isn't efficient enough to do it in a short time period, so I thought maybe there are better ways to do it.

Here is an idea to speed things up, given that the input is only 15 numbers:

Since the numbers are binary, you could represent the board as a bitmask in an integer instead of an array (see wikipedia/masks).
There are only \$2^{15}\$ possible states. You could create a Map<Integer, Integer> from each state bitmask to the next. Then each iteration in your algorithm is gonna be a simple map lookup.

There are more optimizations you could to make it even faster, but this should be enough.


How to make this ASP.NET MVC ViewModel population query efficient? I managed to put together following code to correctly Populate RequestExecutionViewModel model,however this looks extremely verbose to me. I feel some sort of LINQ builtin mapping construct can be used to reduce the over all code size. Please suggest improvements.

string userName = User.Identity.Name;
List<Requests> requests = db.Requests.Where(a => a.UserName.
                          Equals(userName)).ToList();
List<RequestExecutionViewModel> items = new List<RequestExecutionViewModel>();
foreach (Requests request in requests)
{
  RequestExecutionViewModel requestview = new RequestExecutionViewModel();
  requestview.Request = request;
  requestview.ExecutionStatus = db.ExecutionStatus.Where(a =>    a.RequestId.Equals(request.RequestsId)).ToList();
  requestview.Approvals = db.Approvals.Where(a =>  a.RulesId.Equals(request.RulesId)).ToList();
  items.Add(requestview);
}

Edit: With the help of jessehouwing answer I restructured my models and removed the need for RequestExecutionViewModel entirely. Now my query is simplified to this.

var requests = db.Requests.Include(r => r.Rule);

It's hard to tell exactly how to write this query without knowing exactly what your datamodel look like and how you've set it up. An assumption was made you're using Linq 2 SQL, I'll adhere to that assumption unless we hear otherwise. You can eagerly fetch other tables if you tell linq to sql to do so. You use the Include method for that.
If you've set up your datamodel correctly, you should be able to create a query that resembles this:
string userName = User.Identity.Name;
List<RequestExecutionViewModel> = (from request in db.Requests.Include("Requests.ExecutionStatus").Include("Requests.Rules.Approvals");
    where  request.UserName == userName
    select new RequestExecutionViewModel(){
        Request = request,
        ExecutionStatus = request.ExecutionStatus,
        Approvals = request.Rule.Approvals
    }).ToList();

And as you can see, this query instructs Linq-to-SQL to eagerly fetch the to related tables and then uses them in the output shaping process to create your viewmodel directly..
Make sure you look at the profiler to see that this is actually resulting in one query. Linq-to-SQL can at any time decide to loop over the subresults by itself.


Catty: A mini cat clone in Rust As part of my journey in learning the Rust programming language, I decided to make a miniature cat clone (catty) in it. The following is my code, which depends on clap for argument parsing (see below). It currently only supports concatenating 1 file with possibly numbered lines (-n/--number). I tried to be as close as possible to the actual cat program for this:

#[macro_use] extern crate clap;

use std::{io, error, env, fs::read_to_string, path::PathBuf, process};

fn main() {
    process::exit(
        if let Err(err) = cli(env::args().collect::<Vec<_>>()) {
            // CLI parsing errors
            if let Some(clap_err) = err.downcast_ref::<clap::Error>() {
                eprint!("{}", clap_err);
            } else {
                eprintln!("{}", err);
            }
            1
        } else {
            0
        }
    );
}

fn cli(cli_args: Vec<String>) -> Result<(), Box<error::Error>> {
    let matches = clap::App::new("catty")
        .version(crate_version!())
        .about("A minimal clone of the linux utility cat.")
        .arg(clap::Arg::with_name("FILE")
            .help("The file to concatenate to standard output")
            .required(true))
        .arg(clap::Arg::with_name("number")
            .short("n")
            .long("number")
            .help("Numbers all output lines"))
        .get_matches_from_safe(cli_args)?;

    let file_contents = get_file_contents(matches.value_of("FILE").unwrap())?;
    let file_contents: Vec<&str> = file_contents.split("\n").collect();

    let number_lines = matches.is_present("number");

    for (i, line) in file_contents.iter().enumerate() {
        let formatted_line = if number_lines {
            format!("{:>6}  {}", i + 1, line)
        } else {
            line.to_string()
        };

        if i == file_contents.len() - 1 && line.len() > 0 {
            print!("{}", formatted_line);
        } else if !(i == file_contents.len() - 1 && line.len() == 0) {
            println!("{}", formatted_line);
        }
    }

    Ok(())
}

fn get_file_contents(passed_argument: &str) -> Result<String, Box<error::Error>> {
    let mut resolved_path = PathBuf::from(passed_argument);

    if !resolved_path.exists() || !resolved_path.is_file() {
        resolved_path = PathBuf::from(env::current_dir()?);
        resolved_path.push(passed_argument);

        if !resolved_path.exists() || !resolved_path.is_file() {
            return Err(io::Error::new(io::ErrorKind::NotFound, "The passed file is either not a file or does not exist!").into());
        }
    }

    Ok(read_to_string(resolved_path)?)
}

My Cargo.toml is as follows:

[package]
name = "catty"
version = "0.1.0"
authors = ["My Name <my@email.com>"]

[dependencies]

[dependencies.clap]
version = "2.32"
default-features = false
features = ["suggestions"]

Here is what I want to know from this code review:

Is my code idiomatic rust (i.e good error handling, not overly verbose, etc)?
Is my code performant or can it be improved in some way?

4: use std::{env, error, fs::read_to_string, io, path::PathBuf, process};

Just a personal taste, but I would do

use std::{env, error, io, process};
use std::fs::read_to_string;
use std::path::PathBuf;

Yes, nested includes are nice and easy, but hard to extend. It's up to you.

35: let file_contents: Vec<&str> = file_contents.split('\n').collect();

I would suggest using lines instead.

Also, you can omit &str or change the line completly.

Either

let file_contents: Vec<_> = file_contents.lines().collect();

or

let file_contents = file_contents.lines().collect::<Vec<_>>();

46/48: line.len() > 0 / line.len() == 0

Replace that by !line.is_empty() and line.is_empty().

60: resolved_path = PathBuf::from(env::current_dir()?);

Remove PathBuf::from completly, because current_dir is already a PathBuf


Finding permutations of a word This was a basic anagram problem. Take an input word, find all its permutations, then check a dictionary file to see what if any of those are real words.

Dictionary file

My input data:

8
lasting
lameness
severer
lapse
alerting
medical
tasking
crates

My answer:

3 3 3 5 4 4 3 6

The code works slowly, as in, it took maybe a minute.  Aside from any other criticism, I'm curious to the following:

Why are these two so different in speed? Casting as String was much faster:

String current = (String) iter.next();

and

String current = iter.next().toString();

Is there a smarter way than looping through the dictionary every single time for every permutation of a word?
Is there a better data structure for the dictionary than an ArrayList?  If so, why?

    package com.secryption.CA127Anagrams;

import java.util.*;
import java.io.File;

/**
 * Created by bmarkey on 11/26/15.
 */

public class Anagrams {
    public static void main(String[] args) {
        Scanner in = new Scanner(System.in);
        Scanner inFile;
        System.out.println("Enter Data: ");
        int testCases = Integer.parseInt(in.nextLine());
        ArrayList<String> dictionary = new ArrayList<>();

        try {
            File file = new File("/home/me/words.txt");
            inFile = new Scanner(file);

            while (inFile.hasNextLine()) {
                dictionary.add(inFile.nextLine());
            }
            inFile.close();

        } catch (Exception e) {
            e.printStackTrace();
        }

        for (int i = 0; i < testCases; i++) {
            int counter = 0;
            String inputStr = in.nextLine();

            Set<String> allCombos = getPermutations(inputStr);

            Iterator iter = allCombos.iterator();

            while (iter.hasNext()) {
                String current = (String) iter.next();
//              String current = iter.next().toString();
                for( int j = 0; j < dictionary.size(); j++) {
                    if (current.equals(dictionary.get(j))) {
                        counter++;
                    }
                }
            }
            System.out.print((counter - 1) + " ");
        }

    }

    public static Set<String> getPermutations(String str) {
        Set<String> permResult = new HashSet<String>();

        if (str == null) {
            return null;
        } else if (str.length() == 0) {
            permResult.add("");
            return permResult;
        }

        char firstChar = str.charAt(0);
        String rem = str.substring(1);
        Set<String> words = getPermutations(rem);

        for (String newString : words) {
            for ( int i = 0; i <= newString.length(); i++) {
                permResult.add(permCharAdd(newString, firstChar, i));
            }
        }
        return permResult;
    }

    public static String permCharAdd(String str, char c, int j) {
        String first = str.substring(0, j);
        String last = str.substring(j);
        return first + c + last;
    }
}

The algorithm

Given two string \$S\$ and \$Z\$, \$S\$ is a permutation of \$Z\$ if and only if they have the same multiset of characters. The fastest way of checking this is to sort the characters in each of the two strings which will produce two new strings \$S'\$ and \$Z'\$. Finally, if \$S' = Z'\$, \$S\$ is a permutation of \$Z\$. For example: \$S = \text{stop}\$, \$Z = \text{spot}\$, \$S' = Z' = \text{opst}\$.

You can improve the performance:

Create a HashMap<String, Integer>, call it "map",
For each "word" in the word file, do:
    Sort "word" by characters ("fdsbs" -> "bdfss")
    If "word" is not in "map":
        Put "word" into "map" with value "1"
    Else:
        map.put(word, map.get(word) + 1) # Increment count

In order to process an input word, just sort it by characters and do a simple lookup from map. Summa summarum:

import java.util.*;
import java.io.File;
import java.io.FileNotFoundException;

public class Anagrams {

    public static void main(String[] args) {
        Scanner in = new Scanner(System.in);
        System.out.println("Enter number of test cases: ");
        int testCases = Integer.parseInt(in.nextLine());

        File file = new File("/path/to/words.txt");
        Map<String, Integer> map = readFile(file);

        for (int i = 0; i < testCases; i++) {
            String s = sort(in.nextLine());
            System.out.println(map.getOrDefault(s, 0) - 1);
        }
    }

    private static Map<String, Integer> readFile(File file) {
        Map<String, Integer> map = new HashMap<>();

        try (Scanner fileScanner = new Scanner(file)) {
            while (fileScanner.hasNext()) {
                String word = sort(fileScanner.next());

                if (map.containsKey(word)) {
                    map.put(word, map.get(word) + 1);
                } else {
                    map.put(word, 1);
                }
            }
        } catch (FileNotFoundException ex) {
            return null;
        }

        return map;
    }

    private static String sort(String input) {
        char[] chars = input.toCharArray();
        Arrays.sort(chars);
        return new String(chars);
    }
}


Counting how many times each distinct word appears in an input In chapter 3 of Koenig's Accelerated C++, he asks the reader to write a program that performs the task described in the title. At this point, he has only introduced the built-in types, io-types, and vectors, so maps are not allowed. I have written a program that does this, but it is lengthy and requires a number of objects that essentially duplicate existing vectors.

Note: the use of while (cin >> input) will not terminate without an explicit end of line character because of the input type, so a better way to handle the input would be nice to have!

#include <iostream>
#include <vector>
#include <algorithm>
#include <string>

using std::cin;
using std::cout;
using std::vector;
using std::string;
using std::endl;

int main ( int argc, char** argv )
{
    string input;
    vector<string> input_arr;

    cout << "Please enter a sentence or list of words, separated by spaces,"
            "and terminated by an EOL character(^d): ";

    while(cin >> input) {
        input_arr.push_back(input);
    }

    sort(input_arr.begin(), input_arr.end());

    vector<string>::size_type len = input_arr.size();

    uint count = 0;
    vector<uint> count_arr;
    string current_string = input_arr[0];
    vector<string> unique_words;
    unique_words.push_back(input_arr[0]);

    for (vector<string>::size_type i = 0; i < len; i++) {

        count_arr.push_back(0);

        if (current_string != input_arr[i]) {
            count +=1;
            current_string = input_arr[i];
            count_arr.push_back(0);
            unique_words.push_back(input_arr[i]);
        }

        count_arr[count] += 1;
    }

    cout << endl;

    for (int j = 0; j <= count; j++) {
        cout << endl << "The word " << unique_words[j] << " occured " << count_arr[j] << " times!";
    }

    cout << endl;

    return 0;
}

In Python, I could achieve this in about 5 lines, for example:

from collections import defaultdict

print("Please enter some text: ")

input_arr = input().split()
d = defaultdict(int)

for k in input_arr:
    d[k] += 1

for key in d.keys():
    print("The word %s occurs %d times!\n" %(key, d[key]))

I am looking for a better way to accomplish this in C++, preferably not using std::maps, since the author probably put this exercise at this stage in the book for a reason. I'm also just learning C++, do I'd appreciate any advice on best practices that I'm violating in this example.

Old book:

The age of the book starts to get noticable. May be before 2011 it would be still very effective, but not that much now. May be not in 5 lines, but it is possible write it pretty shortly and neatly in upcoming C++17

#include <iostream>
#include <iterator>
#include <vector>
#include <map>

int main() {
    std::vector<std::string> all_words{std::istream_iterator<std::string>(std::cin),
                                       std::istream_iterator<std::string>()};
    std::map<std::string, std::size_t> unique_words;
    for (const auto& word: all_words)
    {
        ++unique_words[word];
    }

    for (const auto& [word, count]: unique_words)
    {
        std::cout << "word " << word << " occured " << count << " times\n";
    }
}

live demo. Be sure to checkout stdin though, I'm not sure if it is preserved.

Unfortunately there are not many books about recent versions of C++, but knowing the basics should be enough to learn new stuff.

It is also possible to write this without using a map, albeit much more sophisticated:

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

int main()
{
    std::vector<std::string> all_words{std::istream_iterator<std::string>(std::cin),
                                       std::istream_iterator<std::string>()};

    std::sort(all_words.begin(), all_words.end());
    auto first_duplicate_pos = std::unique(all_words.begin(), all_words.end());

    for (auto first = all_words.begin(); first != first_duplicate_pos; ++first)
    {
        auto count = std::count(first_duplicate_pos, all_words.end(), *first);
        ++count; //since occured once in non duplicate part
        std::cout << "word " << *first << " occured " << count << " times\n";
    }
}

The idea here is that after using std::unique<>(), the vector is divided into two ranges: one that has only unique elements, and other has duplicates of unique elements, if any. Now, map is not needed, because in fact the first range is map keys with value equal to 1 (remember that unique counts too). Then we just iterate through the other half searching for one of the keys at a time.

Do note that both versions are invincible to the problem mentioned by @jvb. Thus, it is much better to learn to use standard library first, rather than trying to implement it by hand.

Code Review:

int main ( int argc, char** argv )

No need to have arguments if code is not using it.

sort(input_arr.begin(), input_arr.end());

It would be better to just use std::unique<>() after this and count every occurrence of the elements in range.

Using std::endl should be avoided. It flushes the stream, which is pretty slow. Even Bjarne Stroustrup (original creator of the language) avoids that himself. Instead just use '\n'. std::endl is not only portable way to print a newline, but it also incurs overhead.

for (int j = 0; j <= count; j++)

This lines seems dangerous. I was not able to comprehend the counting logic, but usually when <= count appears, it means getting out of range, which is undefined behavior if it would be the whole vector. I'm pretty sure it should be < count. Anyway, it should've been possible to just use range loop and not worry about the comparison operator.

using std::cin;
using std::cout;
using std::vector;
using std::string;
using std::endl;

This probably didn't buy anything, but instead increased amount of code needed to write. I recommend just sticking with std::. If the namespace is too long, for example std::experimental::filesystem, I recommend doing

namespace exfs = std::experimental::filesystem;

at function scope!

update:

People in the comments mentioned that std::unique<>() is not a partitioning algorithm and the right side of the sequence has unspecified values. It is true. I present another solution, which is standard compliant (I actually went on searching if std::unique<>() always performs swap, but it seems like it is not the case).

#include <algorithm>
#include <iterator>
#include <iostream>
#include <functional>
#include <string>
#include <vector>

int main()
{
    std::vector<std::string> all_words(std::istream_iterator<std::string>(std::cin),
                                       {});
    std::sort(all_words.begin(), all_words.end());

    auto first_occurence = all_words.begin();
    while (first_occurence != all_words.end())
    {
        auto next_word_occurence = std::find_if_not(first_occurence, all_words.end(),
                                                    [&first_occurence](const auto& val)
                                                    {
                                                        return val == *first_occurence;
                                                    }
        );

        std::cout << "word " << *first_occurence << " occured "
                  << std::distance(first_occurence, next_word_occurence)
                  << " times\n";
        first_occurence = next_word_occurence;
    }
}

I didn't use std::adjacent_find<>() because it obfuscates the main goal, in my opinion. Using std::find_if_not<>() and its non not version is arguable, but I just went with not version because I usually read on high speeds and sometimes tend to confuse != and ==. Do note that this version is still immune to the point mentioned by @jvb.


Printing a multiplication table I was recently given a coding test by an employer. After the test, this was the employer's response:

  "I’d say that a lot of his solutions were pretty inefficient and took
  a very ‘brute force’ approach to a solution. For example, his solution
  for  question 1 took 30 lines (give or take for some spacing). This is
  normally done in about 8 lines. I did like that he documented all of
  his answers very well,  so that’s a positive, but yeah, just a lot of
  inefficiency throughout his answers  (processing inefficiency, object
  allocation inefficiency, etc)"

I know this question cannot be answered well without an example so to give an example of question 1:

  Write a method that takes a parameter x and then prints out an x by x multiplication table

This is what I wrote:

public void printMultTable(int x)
{
    int totalNumberOfPrints = x * x;
    int rowCounter = 0;
    int columnCounter = 1;
    int number = 0;

    for(int i = 0; i < totalNumberOfPrints; i++)
    {
        if(i < x)
        {
            number = i + 1;
            System.out.print(number + " ");
        }
        else if (i % x == 0)
        {
            columnCounter = columnCounter + 1;
            rowCounter = 2;
            System.out.println();
            System.out.print(columnCounter + " ");
        }
        else
        {
            number = columnCounter * rowCounter;
            System.out.print(number + " ");
            rowCounter = rowCounter + 1;
        }
    }
}

I am experienced, but I read and was told by previous employers that code that is easier to read is better, than complex code that takes only 3 lines. I could have written the code in less lines but I thought this was the way.

So my questions are:

What is processing inefficiency and how is that shown in my code?
What is object allocation inefficiency(I didn't use any static variables)
Is it better to use less lines (I thought it doesn't matter?)

Suggested interviewing technique

When writing code for an interview, always ask questions to seek clarification from your interviewer.  There are nearly always technical decisions to be made, and you want to make choices that match what they have in mind.  (If you don't have any questions, then you probably haven't thought things through properly.)

For this task, I would ask:

"Can I assume that the the input x is a reasonably small positive integer, not requiring validation?  It doesn't make much sense to print anything larger than 100 × 100 anyway, right?"
"I assume that you want the entries to be aligned nicely in a grid?"
"I assume that we don't need row and column headings, and that we don't need to print grid lines?"
"I assume that you want a reasonable balance of simplicity and speed?"

Before you even write a single line of code, you can score points by showing that you know how to negotiate software requirements.

Here's a solution that I would have written:

public static void printMultTable(int x) {
    int width = (int)Math.floor(Math.log10(x * x)) + 2;
    String fmt = "%" + width + "d";
    StringBuilder b = new StringBuilder(x * width);
    for (int row = 1; row <= x; row++) {
        b.setLength(0);
        for (int col = 1; col <= x; col++) {
            b.append(String.format(fmt, row * col));
        }
        System.out.println(b);
    }
}

After writing it, explain and defend any technical decisions:

"To achieve grid alignment, we need to first determine the width of each entry.  String.valueOf(x * x).length() + 1 would have worked, but I think that the mathematical approach is slightly more efficient."
"I could call System.out.print() for every entry, but printing an entire row at once should have less overhead.  Printing the entire table at once might be even faster, but I figured this was good enough."

Note that other answers are possible.  Your interviewer might have answered, "I don't care about any of those requirements.  I just want to know that you know how to write a nested loop."  In that case, just write the simplest code that prints out the right numbers with sloppy formatting.

Your solution and feedback

My biggest concern is that it is not obvious from inspection what your code does.  There are a lot of variables, and the flow of control is convoluted.

I think number is superfluous.  After each assignment, you use it just once.
Why is rowCounter initialized to 0, but columnCounter initialized to 1?  Why is there a rowCounter = 2 assignment right in the middle of everything?  (I would expect reinitializing to either 0 or 1.)  More alarmingly, you increment rowCounter to count the columns printed, and columnCounter to number the current row.  That's the kind of trap that makes co-workers hate your code.
Three cases is excessive.  All of them have System.out.print(… + " "), which makes it cut-and-paste code.  Couldn't you lump the first if(i < x) case with the last case?  (Hint: setting rowCounter = 0 was a poor move.)

The natural way to write this solution is using a nested for-loop.  If you really wanted to do it in one loop, it should be simpler:

public static void printMultTable(int x)
{
    int row = 1, col = 1;
    int totalEntries = x * x;
    for (int i = 1; i <= totalEntries; i++)
    {
        System.out.print((row * col++) + " ");
        if (i % x == 0)
        {
            System.out.println();
            row++;
            col = 1;
        }
    }
}

Note that the function should be static, since there's no reason for it to have any object state.

  I’d say that a lot of his solutions were pretty inefficient and took a very ‘brute force’ approach to a solution. For example, his solution for question 1 took 30 lines (give or take for some spacing). This is normally done in about 8 lines.  I did like that he documented all of his answers very well, so that’s a positive, …

Comparing your solution against the others posted here, I think you would agree that the logic was unnecessarily convoluted.

  but yeah, just a lot of inefficiency throughout his answers (processing inefficiency, object allocation inefficiency, etc)

For all practical purposes, there isn't going to be much user-visible difference in performance of your printMultTable() and the better implementations.  I don't think that there's much object allocation to speak of in this particular solution; that feedback probably referred to the solutions you wrote for the other tasks.  But judging from the way you used a zoo of variables without any clear purpose, I'm inclined to believe their assessment.


Inserting data into database by Python I have to run a python script that enters data into MySQL database table.

My below query runs and inserts data into MySQL database. But I want to optimize it and I have 2 requests:

I want to use try except error handling inside the for loop. Probably before insertstmt or where it would be more efficient.
If there is bug while executing nSql, program should not terminate, rather continue in other nSql tests.
nSql has complex SQL queries with multiple joins, below shown is for simplicity. If any one of the nSql fails during quering, I want the error
message to be displayed.
If the data already exists for yesterday, I want to be deleted it as well.
As the loop iterates, I want data to be deleted if it exists for yesterday as loop iterates through the nSql.

I have:

#! /usr/bin/env python
import mysql.connector
con=mysql.connector.connect(user='root', password ='root', database='test')
cur=con.cursor()

sel=("select id, custName, nSql from TBLA")
cur.execute(sel)
res1=cur.fetchall()

for outrow in res1:
    print 'Customer ID : ', outrow[0], ': ', outrow[1]
    nSql = outrow[2]
    cur.execute(nSql)
    res2=cur.fetchall()

    for inrow in res2:
        dateK =inrow[0]
        id= inrow[1]
        name= inrow[2]
        city=inrow[3]
        insertstmt=("insert into TBLB (dateK, id, name, city) values ('%s', '%s', '%s', '%s')" % (dateK, id, name, city))
        cur.execute(insertstmt)

con.commit()
con.close()

Database schema is:

create table TBLA (id int, custName varchar(20), nSql text);

insert into TBLA values ( 101, 'cust1', "select date_sub(curdate(), interval 1 day) as dateK, id, 'name', 'city' from t1"), ( 102, 'cust2', "select date_sub(curdate(), interval 1 day) as dateK, id, 'name', 'city' from t2"), ( 103, 'cust3', "select date_sub(curdate(), interval 1 day) as dateK, id, 'name', 'city' from t3");

create table t1 (id int, name varchar(20), city varchar(20));
create table t2 (id int, name varchar(20), city varchar(20));
create table t3 (id int, name varchar(20), city varchar(20));

insert into t1 values( 101, 'bob', 'dallas'), ( 102, 'boby', 'dallas');

insert into t2 values( 101, 'bob', 'dallas'), ( 102, 'boby', 'dallas');

insert into t3 values( 101, 'bob', 'dallas'), ( 102, 'boby', 'dallas');

create table TBLB (dateK date, id int, name varchar(20), city varchar (20));

OK, here's the simpliest version of try:except: block for your case:

# some code
for inrow in res2:
    # some code
    try:
        cur.execute(insertstmt)
    except MySQLdb.ProgrammingError:
        pass

That's pretty much it. You probably want to know which queries failed, so you should use for example that version:

    try:
        cur.execute(insertstmt)
    except MySQLdb.ProgrammingError:
        print "The following query failed:"
        print insertstmt

Now as for the other question. You have to use a delete query. For example:

DELETE FROM TBLB WHERE dateK < CURDATE();

or something like that (note that CURDATE is a built-in MySQL function). Read more about delete queries here:

http://dev.mysql.com/doc/refman/5.0/en/delete.html


Working out statistics for unique users The below code is run on a routine rather than say login time but being new to using FETCH NEXT I can't help think maybe there is a trick here to merge at least one of the SET lines.

The end results should give me the total unique users, how many completed a module and how many passed.

Fields are indexed on the table at this time but all additional thoughts welcome.

 USE [DB]
    GO
    SET ANSI_NULLS ON
    GO
    SET QUOTED_IDENTIFIER ON
    GO
    ALTER PROCEDURE [dbo].[spModuleStatisticsUnique2]
    -- Add the parameters for the stored procedure here
    AS

    DECLARE @i INT = 0;
DECLARE @i INT = 0;
DECLARE @Started   int;
DECLARE @Completed  int;
DECLARE @Passed  int;
DECLARE @ModuleID  int;
    DECLARE merge_cursor CURSOR FAST_FORWARD FOR SELECT [ModuleID] FROM dbo.[TblModules]
    --ORDER BY [ModuleID]
    OPEN merge_cursor

    FETCH NEXT FROM merge_cursor INTO @ModuleID

    WHILE @@FETCH_STATUS = 0
    BEGIN

        SET @Started =  (Select Count(ID) FROM [TblResults] WHERE [ModuleID] = @ModuleID)
        SET @Completed = (Select Count(ID) FROM [TblResults] WHERE [ModuleID] = @ModuleID and ModuleDatecomplete <> '')
        SET @Passed = (Select Count(ID) FROM [TblResults] WHERE [ModuleID] = @ModuleID and Pass = 'Yes')

        UPDATE [TblModules]
        SET
        [Started] = @Started
        ,[Completed] = @Completed
        ,[Passed] = @Passed
        ,[PassedPercent] = ((@Passed * 100.0) / @Started)
        WHERE
        [ModuleID] = @ModuleID

    FETCH NEXT FROM merge_cursor INTO @ModuleID
    END

    CLOSE merge_cursor
    DEALLOCATE merge_cursor

I don't think you need a cursor, looks like a simple update:

 WITH cte AS
  ( SELECT -- do all the counts using conditional aggregation
       ModuleID
       ,Count(DISTINCT ID) AS Started
       ,Count(DISTINCT CASE WHEN ModuleDatecomplete <> '' THEN ID END) AS Completed
       ,Count(DISTINCT CASE WHEN Pass = 'Yes' THEN ID END) AS Passed
    FROM TblResults
    GROUP BY ModuleID
  )
 UPDATE TblModules
 FROM cte
 SET
   Started = cte.Started
   ,Completed = cte.Completed
   ,Passed = cte.Passed
   ,PassedPercent = ((cte.Passed * 100.0) / cte.Started)
 WHERE
    ModuleID = cte.ModuleID

Of course this is fully untested, but the syntax should be ok as-is.

Edit:

In 99+% rewriting a cursor with set-based SQL improves performance.
In your case you run three Selects and one Update per row, e.g. for 100 rows returned by the cursor Select the DBMS needs to execute 400 statements sequentially, 300 Selects and 100 Updates. Combining the three SETs into one using conditional aggregation would reduce the amount of work to 100 Selects and 100 Updates and result in approx. half the runtime. But a single Update might run 10 or 100 or 1000 times faster.


Zip file using yauzal lib I'm using the following code to unzip file which is working OK.
I'm getting file with the request (express) and  unzip it in specified folder

_unzip: function (req) {
        return new Promise((resolve, reject) =>{
            var noOfFiles = 0;
            let filePath = path.join(req.file.destination, req.file.filename);
            logger.info("Unzip filePath: " + filePath);
            if (req.file) {
                yauzl.open(filePath, function (err, zipFile) {
                    if (err) throw err;
                    zipFile.on('entry', (entry) =>{
                        //console.log(entry.fileName);
                        if (/\/$/.test(entry.fileName)) {
                            return;
                        }
                        zipFile.openReadStream(entry, (err, readStream) => {
                            if (err) {
                                logger.info(err);
                                reject(err);
                            } else {
                                // ensure parent directory exists, and then:
                                let destinationFileName = "./" + entry.fileName;

                                let directoryName = path.dirname(destinationFileName);
                                if (!fs.existsSync(directoryName)) {
                                    mkdirp(directoryName, (err) =>{
                                        if (err) {
                                            logger.info(err);
                                            reject(err);
                                        } else {
                                            writeIntoFile(readStream, destinationFileName, reject);
                                            noOfFiles++;
                                        }
                                    });
                                } else {
                                    writeIntoFile(readStream, destinationFileName, reject);
                                    noOfFiles++;
                                }
                            }
                        });
                    }).once('error', (err) =>{
                        logger.info(err);
                        reject(err);
                    }).once('close', () =>{
                        logger.info("Unpacked " + noOfFiles + " files");
                        resolve();
                    });
                });
            }
    });
}

https://github.com/thejoshwolfe/yauzl

Check for req.file before using its properties
Return/reject early instead of nesting a big else clause
Since you're using ES2015 syntax do it consistently:

template strings via backticks
arrow functions in callbacks that don't utilize this of the callee
string methods with self-explanatory names like endsWith instead of regexps
let instead of var (there was one in your code)

Deduplicate reject+logger combo and writeIntoFile
Use single or double quotes consistently

return new Promise((resolve, reject) => {
    if (!req || !req.file) {
        __rejectAndLog('No file provided');
        return;
    }
    let numberOfFiles = 0;
    let filePath = path.join(req.file.destination, req.file.filename);
    logger.info(`Unzip filePath: ${filePath}`);

    yauzl.open(filePath, (err, zipFile) => {
        if (err) {
            __rejectAndLog(err);
            return;
        }
        zipFile.on('entry', entry => __processEntry(zipFile, entry))
            .once('error', __rejectAndLog)
            .once('close', () => {
                logger.info(`Unpacked ${numberOfFiles} files`);
                resolve();
            });
    });

    function __processEntry(zipFile, entry) {
        if (entry.fileName.endsWith('/')) {
            return;
        }

        zipFile.openReadStream(entry, (err, readStream) => {
            if (err) {
                __rejectAndLog(err);
                return;
            }

            let destinationFileName = './' + entry.fileName;
            let directoryName = path.dirname(destinationFileName);
            if (fs.existsSync(directoryName)) {
                __writeFile();
                return;
            }

            mkdirp(directoryName, (err) => {
                if (err) {
                    __rejectAndLog(err);
                } else {
                    __writeFile();
                }
            });

            function __writeFile() {
                writeIntoFile(readStream, destinationFileName, reject);
                numberOfFiles++;
            }
        });
    }

    function __rejectAndLog(err) {
        logger.info(err);
        reject(err);
    }
});

entry-processing function is extracted to make the zipFile flow more obvious
__ prefix is used for subfunctions following your naming of _unzip


Constructor function, prototype and making a modal I'm playing around with prototypes, constructor functions and javascript - I'm not really a Javascript developer but am interested in this.

Am I on the right track with this code? I'm looking for a code review from a JS developer. I know the codes a bit funky.

I created an example of it working here.

function modal( width, height, backgroundColor, overlayColor, showOverlay, url )
{
    this.width = width; // a percentage based number %
    this.height = height; // optional, usually needs not to be set
    this.backgroundColor = backgroundColor; // set the background colour to the modal
    this.overlayColor = overlayColor; // as hex, to be converted to rgba
    this.url = url;
    this.showOverlay = showOverlay;
    this.createModal = function()
    {
        // create elements to add classes to and insert in to dom
        var modalOverlay = document.createElement('div')
        ,   modalContent = document.createElement('div')
        ,   modalInner = document.createElement('div');

        // build html structure
        modalOverlay.className = "modalOverlay";
        modalContent.className = "modalContent";
        modalOverlay.appendChild(modalContent);
        modalInner.className = "modalInner";
        modalContent.appendChild(modalInner);

        // add new html structure to body
        document.body.appendChild(modalOverlay);

        // Theme model
        this.themeModel();
    }
    this.themeModel = function() {

        // is there a background color?
        var overlayBgColor = this.showOverlay ? this.convertHext(this.overlayColor,50) : "transparent";

        $('.modalOverlay').css({
            'background-color': overlayBgColor
        });

        $('.modalContent').css({
            'background-color': this.backgroundColor,
            'width': this.width,
            'height': this.height
        });

        // load content
        this.loadContent();
    }
    this.loadContent = function(){
        if( url )
        {
            $('.modalInner').load(url);
        }
        else
        {
            $('.modalInner').html("please provide a url to load! close");
        }

    }
    this.showModal = function(){
        this.createModal();
        $('.modalOverlay').fadeIn(50);
        $('body').addClass('modalShow');
    }
    this.hideModal = function(){
        $('.modalOverlay').fadeOut(300, function(){
           $('.modalOverlay').remove();
        })
        $('body').removeClass('modalShow');
    }
    this.convertHext = function(hex,opacity)
    {
        hex = hex.replace('#','');
        r = parseInt(hex.substring(0,2), 16);
        g = parseInt(hex.substring(2,4), 16);
        b = parseInt(hex.substring(4,6), 16);
        result = 'rgba('+r+','+g+','+b+','+opacity/100+')';
        return result;
    }
}
body
{
    position: relative;
    margin: 0; /* reset margin */
    padding: 0; /* reset padding */
}

.modalShow .modalContent
{
    opacity: 1;
   overflow':'hidden';
  -webkit-transition: all .3s ease-out;
          transition: all .3s ease-out;
margin-top: 30px;
}

.modalOverlay
{
    display: none;
    overflow: auto;
    overflow-y: scroll;
    position: fixed;
    top: 0;
    right: 0;
    bottom: 0;
    left: 0;
    z-index: 9999;
    -webkit-overflow-scrolling: touch;
    outline: 0;
    background-color: #2d2d2d;
    background-color: rgba(45,45,45,0.2);

}
.modalContent
{
    width: 50%;
    height: auto;
    background-color: #FFFFFF;

    margin-top: 0;
    margin-bottom: 30px;
    margin-left: auto;
    margin-right: auto;
    opacity: 0;
  -webkit-transition: all .3s ease-out;
          transition: all .3s ease-out;
}

.modalInner
{
    padding: 20px;
}

    show modal

Here's some thoughts

function modal(width, height, backgroundColor, overlayColor, showOverlay, url) {

One problem some programmers rant with this is that if you forget a parameter, or forget the order. This would seriously impale your code. An alternative is to pass an object that contains the config.

new Modal({
  width : 100,
  height : 100
});

One advantage of this over the previous is that you become more verbose and don't need to remember the order of the params. Additionally, you can use $.extend to place in defaults, if they didn't exist on options.

function Modal(options){
  options = $.extend(options,{
    width : /*default width */,
    height : /*default height */
  });
  ...
}

this.createModal = function () {...};
this.themeModel = function () {...};

You lose the advantage of prototypal inheritance. Better put methods on the prototype so they are shared. Advantage is you save memory by not duplicating functions per instance. Disadvantage is you lose emulated private property - but everything is accessible one way or another, no point using privates.

$('.modalOverlay').css({
    'background-color': overlayBgColor
});

$('.modalContent').css({
  'background-color': this.backgroundColor,
  'width': this.width,
  'height': this.height
});

Suggesting you move this to CSS by adding/removing classes. That way, proper separation of concerns is attained. You don't want to be debugging and 1 hour later, realize that the styles were not in the CSS files but in JS files.

To clarify, this kind of code will bite back in the long run. Suggesting you add a class to the top-most element of your modal HTML, and then style relative to it. You can provide a custom class name to the constructor instead of the styles.

.blackish-modal a{/*style for links on modal with blackish-modal class*/}
.whitish-modal a{/*same goes, but for whitish-modal class*/}

$('.modalInner').load(url);

IMO, $.fn.load is just a convenience function. Suggesting you use the lower-level AJAX functions for more flexibility. But as a convenience function, if it works for you, that's fine.

result = 'rgba(' + r + ',' + g + ',' + b + ',' + opacity / 100 + ')';

String cocatenation in JS is really messy with all that quotes. Here's an alternative:

var rgba = [r,g,b,opacity/100].join(',');
result = 'rgba(' + rgba + ')';

var delegateLoginModal = new modal("90%", "", "", "", true, "");

// create your events
$('body').on('click', '.showModal', function (e) {
    e.preventDefault();
    delegateLoginModal.showModal();
});

Suggesting you look at how Bootstrap implemented their modal. It's in a form of a jQuery plugin. Your code looks cleaner if it did something as clean as:

$('someElement').modal(/*config|method*/);


Merge Sort in Kotlin I started learning kotlin recently,with some background in java.

So I wrote a merge sort algorithm in kotlin.
I am looking for best practices in kotlin and looking for this code review.

I also wanted to know about its efficiency and ways to increase it.

Thanks

fun merge(list: List<Int>,lo: Int,hi: Int): List<Int>{
    val mid=(lo+hi)/2
    var i=lo
    var j=mid+1
    var new_list=mutableListOf<Int>()
    for(k in 0..(lo-1)) new_list.add(list[k])
    while(!(i==mid+1 && j==hi+1)){
        if(i==mid+1){
            new_list.add(list[j])
            j++
        }
        else if(j==hi+1){
            new_list.add(list[i])
            i++
        }
        else if(list[i]>list[j]){
            new_list.add(list[j])
            j++
        }
        else{
            new_list.add(list[i])
            i++
        }
    }
    for(k in (hi+1)..(list.size-1)) new_list.add(list[k])
    return new_list.toList()
}

fun mergeSort(list: List<Int>,lo: Int,hi: Int): List<Int>{
    if(lo<hi){
        val mid=(lo+hi)/2
        val list1=mergeSort(list,lo,mid)
        val list2=mergeSort(list1,mid+1,hi)
        val list3=merge(list2,lo,hi)
        return list3
    }
    return list
}

I have zero experience with Kotlin, but I can tell you how to make your implementation run faster. You could allocate the auxiliary space only once throughout the entire algorithm, and you essentially merge from one array to other, then at a higher recursion level, you swap their roles and merge in opposite direction. You can learn about this approach here.

Also, one nitpick: in most languages (I know), the sorting method/function does not return any list; they just modify the input array.

Hope that helps.


How to use string constants correctly I often get into the dilemma of how to use string constants, this is the way I use now:
UIImage *image = [UIImage imageNamed:@"apple.png"];
NSArray *imageNames = @[@"apple01.png",@"apple02.png"];
NSDictionary *imageKeyValues = @{@"Avatar" : @"apple.png"};

Once I encounter this type of code, I often use string pointers to refactor, because I don't know what the string code means:
static NSString *appleImage = @"apple.png";
UIImage *image = [UIImage imageNamed:appleImage];

NSArray *imageNames = @[@"apple01.png",@"apple02.png"];

static NSString  *avatarKey = @"Avatar";
NSDictionary *imageKeyValues = @{avatarKey : appleImage};

My question now is: Should I assign each string to a variable? Many strings are not shared and will only be used once. Is it worth modifying like this? Is there a better way?

You ask:

Should I assign each string to a variable? Many strings are not shared and will only be used once.

Obviously, in the other case, where the string is reused/shared, then of course one would (and should) define the constant/pointer once rather than ever repeating the same string literal in multiple places. This makes code easier to maintain.
You might also use constants/pointer to centralize the definition of all the possible keys in one place. E.g. if the string literals define possible key names used in some web interface, we might want to put all these key names in one place, making it easier to keep track of the API.
But if the strings are not shared and there is no organizational benefit to centralizing these definitions, then no, one would not assign each string its own variable.

Is it worth modifying like this?

If you are asking whether you should convert this ...
- (UIImage *)appleImage {
    UIImage *image = [UIImage imageNamed:@"apple.png"];
    return image;
}

... to the following ...
- (UIImage *)appleImage {
    static NSString *name = @"apple.png";
    UIImage *image = [UIImage imageNamed:name];
    return image;
}

... then the answer is, no, there is little benefit in doing do.
I would advise against pulling the string literal into its own local NSString pointer just for the sake of it. It just adds syntactic noise in this case. Yes, there are cases where one might pull the literal into its own line for the sake of clarity, but that not is not the case here.
The governing principles are code maintainability and readability.


Project Euler #6 in Java Project Euler #6:

  The sum of the squares of the first ten natural numbers is,

  \$1^2+2^2+ ... + 10^2 = 385\$

  The square of the sum of the first ten natural numbers is,

  \$(1+2+ ... + 10)^2 = 55^2 = 3025\$

  Hence the difference between the sum of the squares of the first ten natural numbers and the square of the sum is \$3025 − 385 = 2640\$.

  Find the difference between the sum of the squares of the first one hundred natural numbers and the square of the sum.

Here is my solution:

public class DifferenceFinder {

    private static final int MAX = 100;

    public static void main(String[] args) {
        long time = System.nanoTime();
        int result = MAX * (MAX + 1) / 2;
        result *= result;
        for(int i = 1; i <= MAX; i++) {
            result -= i * i;
        }
        time = System.nanoTime() - time;
        System.out.println("Result: " + result
                + "\nTime used to calculate in nanoseconds: " + time);
    }

}

It simply does:

$$(1+2+ ... + 100)^2-1^2-2^2- ... -100^2$$

Output:

  Result: 25164150
Time used to calculate in nanoseconds: 2231

Questions:

Is the simplest solution the most efficient one?
Does it smell?

A sum of first \$n\$ numbers is \$\frac{n(n+1)}{2}\$. A sum of squares of first \$n\$ numbers is \$\frac{n(n+1)(2n+1)}{6}\$. A difference in question is \$\frac{n^2(n+1)^2}{4} - \frac{n(n+1)(2n+1)}{6} = \frac{(n-2)(n-1)n(n+1)}{12}\$.

This solution is likely the most efficient.


Code to initialize, set, or get node properties I have this JavaScript excerpt, and I was wondering if this (primarily the usage of switch case) would be considered good practice. One item of note is that I have it separated into two similar parts to not evaluate the same boolean check over and over again when it is static the whole time.

if (strict) {
  for (var i = 0, len = node.properties.length; i < len; ++i) {
    var other = node.properties[i];
    if (other.key.name !== prop.key.name) continue;
    var otherKind = other.kind;
    switch (kind) { // used as a kind of goto
    case "init":
    case "get":
    case "set":
      if (otherKind === "get" ||
          otherKind === "set" ||
          otherKind === "init")
        raise(prop.key.start, "Redefinition of property");
  }
} else if (sawGetSet)) {
  loop:
  for (var i = 0, len = node.properties.length; i < len; ++i) {
    var other = node.properties[i];
    if (other.key.name !== prop.key.name) continue;
    var otherKind = other.kind;
    switch (kind) { // used as a kind of goto
    case "get":
    case "set":
      if (otherKind === "init") break;
    case "init":
      if (otherKind === "get" || otherKind === "set") break;

    default: continue loop; // skip the below portion, don't throw error
    }
    raise(prop.key.start, "Redefinition of property");
  }
}

Should I rewrite it like this, something else, or leave it as is? (Speed still is important.)

if (strict) {
  for (var i = 0, len = node.properties.length; i < len; ++i) {
    var other = node.properties[i];
    if (other.key.name !== prop.key.name) continue;
    var otherKind = other.kind;
    if ((kind === "init" ||
         kind === "get" ||
         kind === "set") &&
        (otherKind === "get" ||
         otherKind === "set" ||
         otherKind === "init")) {
      raise(prop.key.start, "Redefinition of property");
    }
  }
} else if (sawGetSet)) {
  for (var i = 0, len = node.properties.length; i < len; ++i) {
    var other = node.properties[i];
    if (other.key.name === prop.key.name) continue;
    var otherKind = other.kind;
    if ((kind === "get" ||
         kind === "set") &&
        (otherKind === "init" ||
         otherKind === "get" ||
         otherKind === "set")) {
      raise(prop.key.start, "Redefinition of property");
    } else if ((kind === "init") &&
               (otherKind === "get" ||
                otherKind === "set")) {
      raise(prop.key.start, "Redefinition of property");
    }
  }
}

For one, I'm wondering about strict vs sawGetSet, and why it's an if...else if structure (instead of just using one of the two variables, and an if...else). But without knowing the rest of the code, I won't get into that.

Anyway, I see no need for switch statements. I don't know if I'd call it "bad practice", but it is certainly strange practice to use a switch when it's not really called for.

I also doubt you gain any appreciable performance by splitting the code into two blocks. At least, I'd advice you to thoroughly benchmark it before duplicating code; premature optimization is the root of all evil, and all that.

As for the logic, it seems you're only interested in 'get', 'set' and 'init' values (I'll call these "the three magic values" in the following), and you call raise() for certain combinations of those. Anything else passes through.

So first, I would move the checking of kind outside the loop, since you can skip everything in your code, if kind isn't one of the three magic values.

Now, if the idea is that kind is guaranteed to always be one of the three magic values, then you can obviously skip that check. But then there's no reason to check its value; you only have to check the other.kind value. So either way, you can skip a bunch of code.

Your code is written to check for kind/otherKind-combinations that'll call raise(), but since that's most combinations, it'd be simpler to turn it inside out, and focus on the combinations that won't call raise().

As far as I can tell, the only configurations that won't call raise() (besides a key.name mismatch) are:

if kind is not any of the three magic values
if other.kind is not any of the three magic values
if

sawGetSet is true, and
kind is 'init', and
other.kind is also 'init'

Everything else will call raise().

So those 3 configurations are the only ones we need to check, and if any of them come up true, we can just skip everything else. If none come up true, call raise().

So from what I can gather, this should have the same result

var i, l,
    kindIsInit, kindIsMagic,
    other, otherIsInit, otherIsMagic;

// we'll be using this more than once
kindIsInit = (kind == 'init');

// Check for configuration #1
kindIsMagic = kindIsInit || (kind === 'get') || (kind === 'set');
if(!kindIsMagic) {
  // I imagine a `continue`, `break` or `return` could go here;
  // I've just put `return` as an example.
  return;
}

for(i = 0, l = node.properties.length; i < l; ++i) {
  other = node.properties[i];
  if(other.key.name !== prop.key.name) {
    continue;
  }

  // we might be using this more than once, too
  otherIsInit = (other.kind === 'init');

  // Check for configuration #3
  // (I'm checking this before #2 because it's probably infinitesimally
  // faster, but, really, the order of 2 & 3 really doesn't matter)
  if(sawGetSet && kindIsInit && otherIsInit) {
    continue;
  }

  // Check for configuration #2
  otherIsMagic = otherIsInit || (other.kind === 'get') || (other.kind === 'set');
  if(!otherIsMagic) {
    continue;
  }

  // If we're here, both kind and other.kind are one of the three "magic"
  // values, and any remaining combination of them should call raise()
  raise(prop.key.start, "Redefinition of property");
}

That should be both faster and simpler than what you have. "Simpler" is of course relative, and you should still copiously document all this, since it'll be tricky to read regardless.

For fun, here's a probably slower (haven't tried it), but more readable (I think) version

// helper function
function parseKind(kind) {
  var isInit = (kind == 'init');
  return {
    init: isInit,
    magic: isInit || (kind === 'get') || (kind === 'set')
  };
}

var i, l, other, kind = parseKind(prop.kind); // I'm guessing `kind` in your code is a shortcut for `prop.kind`.

// Config #1
if(!kind.magic) { return; }

for( i = 0, l = node.properties.length ; i < l ; i++ ) {
  if(node.properties[i].key.name !== prop.key.name) { continue; }

  other = parseKind(node.properties[i].kind);

  // Config #2
  if(!other.magic) { continue; }

  // Config #3
  if(sawGetSet && kind.init && other.init) { continue; }

  raise(prop.key.start, "Redefinition of property");
}


Umbraco Examine Search for Full Site I have a macroscript which reads a query string for search term parameters, and then does a multiple word GroupedOr search through UmbracoExamine / Lucene.NET.  Because this is a full site search, I have hard-coded an array of about 100 fields across the site to allow the user to search against pretty much anything on the site.

Is there any way to make this search more efficient?  Currently I'm averaging about 5 seconds (per 10,000 iterations on stopwatch) to search 100 fields on about ~2.2k nodes and returning ~600 results.

NOTE: I have removed the HTML / Inner loop code as I've already determined that this area isn't where the search is hanging.

string SearchTerm = string.Empty;
if (!string.IsNullOrEmpty(Request.Form["SearchTerm"]))
{
    SearchTerm = Request.Form["SearchTerm"];
    string[] SearchTermList = SearchTerm.Split(new char[] { ' ' }, StringSplitOptions.RemoveEmptyEntries);
    string[] aliasList = new string[]{"umbracoUrlName", "menuText", "pageHeading", "pageSubheading",
        "bodyText", "alerts", "headerQuote", "headerQuoteAttribution", "officePhone", "officeFax",
        "officeEmail", "officeAddress", "officeCityofficeState", "officeZip", "officeAddress2",
        "officeName", "navSummary", "latestJournals", "frequentNumbers", "searchIntro", "introduction",
        "heading", "introText", "smName", "divisionTitle ", "districtTitle ", "districtNotes ",
        "authorFirst ", "authorLast ", "articleTitle ", "articleKeywords ", "associatedJournal ",
        "headline ", "content", "ruleSection", "articleCategories", "staffListing", "categoryTitle",
        "articleAuthor", "articleCategory", "articleEndnotes", "journalVolume", "journalIssue",
        "journalDate", "ruleCategory", "caseNotes", "proposedOpinion", "opinionAdopted", "opinionSummary",
        "opinionEndnotes", "secondarySegment", "primaryTitle", "primarySubtitle", "primaryIntro",
        "primaryNote", "secondaryTitle", "secondarySubtitle", "secondaryIntro", "secondaryNote",
        "contactName", "contactTitle", "contactPhone", "contactEmail", "contactLabel", "contactIntro",
        "orgMembers", "faqQuestion", "faqAnswer", "eventSummary", "eventVenue", "eventAddress1",
        "eventAddress2", "eventCity", "eventState", "eventZip", "eventStartDateTime", "eventEndDateTime",
        "eventContactName", "eventContactTitle", "eventContactEmail", "eventContactPhone", "groupMembers",
        "pluralName", "makePlural", "subMembers", "newsDate", "newsLocation", "newsSummary", "newsCategory"};

    BaseSearchProvider collection = ExamineManager.Instance.SearchProviderCollection["SiteSearchSearcher"];
    ISearchCriteria searchCriteria = collection.CreateSearchCriteria(IndexTypes.Content, BooleanOperation.Or);
    ISearchCriteria filter = searchCriteria.GroupedOr(aliasList, SearchTermList).Compile();
    var SearchResults = collection.Search(filter).OrderByDescending(n => n.Score);
    string resultText = SearchResults.Count() == 1 ? "result" : "results";

    if (SearchResults.Count() > 0)
    {
      //Removed HTML / Printing Nodes & URL's
    }
}

From my point of view, I cannot see any further improvement in terms of efficiency when you have a massive list of fields like that.

However, you might be interested in change the logic slightly.

For example, you might want the user to select the topic that he is interested in (using a dropdown list):

Offices
News
...
Events

And once he had selected the topic, and typed the keyword (s) in the input field, you can perform the search against an array which contains the fundamental fields such as heading, bodyText and against a custom list of fields just for that particular topic, such as the News:

string[] aliasNewsList = new string[]{"newsDate", "newsLocation", "newsSummary", "newsCategory"};