Activity-based permission checking I threw together this Python function to check if a User has a certain Permission.

First of all, Permissions have this kind of format: category1.category2.some.task

and can also include wildcards like this: app.view.*

All the permissions a User has are stored in a list, so for example:

["app.cat1.*", "app.cat2.do.something", "app.cat2.cat3.*"]

You can then use the function below to check if a User has a certain Permission in his list:

def check_permission(needed_permission, permissions):
    #normalise needed permissions ending with a "." dot, convert filter to list again!
    needed_permissions_list = list(filter(None, needed_permission.split(".")))
    #loop over the permissions the user has, check if each matches the needed one until we find one and return True
    for permission in permissions:
        #permission is the permission the user already has and we are working on
        #permission_list is the list representation of the permission mentioned above
        permission_list = permission.split(".")
        #if the needed permission is exactly the same as the one we are working on, return True. E.g. needed: app.*, in list: app.*
        if permission_list == needed_permissions_list:
            return True
        #needed permission nearly matches, not the same because of wildcard. Return True
        if permission_list[-1] == "*" and (permission_list[:-1] == needed_permissions_list):
            return True
        #zip the to lists together to iterate
        zipped = zip(permission_list, needed_permissions_list)
        #iterate over the two lists simultaneousley
        for item, item2 in zipped:
            #whoa, an item doesn't match, time for further checks
            if item != item2:
                if item == "*":
                    #it's not matching because it's a wildcard, so return True
                    return True
                else:
                    #it's not a wildcard, so it's definetely not the permission we are looking for
                    break
                return True
    #nothing matched, return False. Permission we're looking for not in the list
    return False

permissions = ["app.cat1.*", "app.cat2.do.something", "app.cat2.cat3.*"]
assert check_permission("app.cat2.do.something", permissions) == True
assert check_permission("app.cat2.do.something.special", permissions) == False
assert check_permission("app.cat1.some.task", permissions) == True
assert check_permission("app.cat2.cat4", permissions) == False
assert check_permission("app.cat2.cat3.do.something.special", permissions) == True
assert check_permission("something.completely.different", permissions) == False

This is completely unnecessarily complicated:

needed_permissions_list = list(filter(None, needed_permission.split(".")))

This is equivalent:

needed_permissions_list = needed_permission.split(".")

The return True at the end of the if item != item2 is unreachable,
so you can delete that line.

item2 is not a good name. needed_item would be better, so you don't mix up which is which. It's important to know which is which, because the needed item must not contain *.

This is unnecessary:

    if permission_list[-1] == "*" and (permission_list[:-1] == needed_permissions_list):
        return True

This treats a special case, that the rest of the method automatically handles.
You can safely remove this part.

This is a shorter and simpler implementation:

def check_permission(needed_permission, permissions):
    needed_permissions_list = needed_permission.split(".")
    for permission in permissions:
        permission_list = permission.split(".")
        if permission_list == needed_permissions_list:
            return True
        zipped = zip(permission_list, needed_permissions_list)
        for item, needed_item in zipped:
            if item != needed_item:
                if item == "*":
                    return True
                else:
                    break
    return False

The assertions are not written in a Pythonic way:
you should avoid comparisons with True and False.
This is the Pythonic way:

assert check_permission("app.cat2.do.something", permissions)
assert not check_permission("app.cat2.do.something.special", permissions)
assert check_permission("app.cat1.some.task", permissions)
assert not check_permission("app.cat2.cat4", permissions)
assert check_permission("app.cat2.cat3.do.something.special", permissions)
assert not check_permission("something.completely.different", permissions)

Your spec is not very clear.
For example you didn't mention if the pattern a.*.c is valid.
Probably not valid, because your code doesn't handle the case (for example a.b.x will yield true).
Effectively, a pattern like a.*.c is handled as if it was a.*.
It's good to specify this clearly.

I don't really like this special treatment:

    if permission_list == needed_permissions_list:
        return True

Because this check has an overlap with the logic in the for loop:
if the two lists are not identical,
the loop will repeat some of the checks.
It would be better to get rid of this overlap,
see this alternative implementation:

def check_permission2(needed_permission, permissions):
    needed_permissions_list = needed_permission.split(".")
    for permission in permissions:
        permission_list = permission.split(".")
        zipped = zip(permission_list, needed_permissions_list)
        for item, needed_item in zipped:
            if item != needed_item:
                if item == "*":
                    return True
                else:
                    break
        else:
            return len(permission_list) == len(needed_permissions_list)
    return False

Finally,
another alternative implementation that also passes your assertions and somewhat simpler, taking a different approach without splitting to segments:

def check_permission(needed_permission, permissions):
    for permission in permissions:
        index_of_star = permission.find('*')
        if index_of_star == -1:
            if needed_permission == permission:
                return True
        else:
            normalized = permission[:index_of_star]
            if needed_permission.startswith(normalized):
                return True
    return False


Extract numbers from an AlphaNumeric string I have an alphanumeric string like sdff45hg589>@#DF456&<jk778P&&FHJ75, of which I want to extract only the numbers, without using regex or any other functions.

Dim input As String = "sdff45hg589>@#DF456&<jk778P&&FHJ75"
Dim output As String = New String((From c As Char In input Select c Where Char.IsDigit(c)).ToArray())
MsgBox(output) 'Display the output

The message box will display 4558945677875.

For extracting letters from the same string, I use the following:

 Dim input As String =  "sdff45hg589>@#DF456&<jk778P&&FHJ75"
 Dim output As String = New String((From c As Char In input Select c Where Char.IsLetter(c)).ToArray())
 MsgBox (output)

Is this a better way to extract both?

If you want to extract both at the same time (I don't know if it is a requirement, it isn't clear in the question) you should iterate once through each character to place them in different lists.

  Dim input As String = "sdff45hg589>@#DF456&<jk778P&&FHJ75"
  Dim characters As List(Of Char) = New List(Of Char)()
  Dim numbers As List(Of Char) = New List(Of Char)()
  For Each c As Char In input
     If (Char.IsNumber(c)) Then
        numbers.Add(c)
     ElseIf (Char.IsLetter(c)) Then
        characters.Add(c)
     End If
  Next

'Do whatever you have to do here

Edit

As pointed out @mjolka, since the goal of the user is to show the splitted numbers and letters, a StringBuilder would be more appropriate. Here is my updated answer :

  Dim input As String = "sdff45hg589>@#DF456&<jk778P&&FHJ75"
  Dim characters As StringBuilder = New StringBuilder()
  Dim numbers As StringBuilder = New StringBuilder()
  For Each c As Char In input
     If (Char.IsNumber(c)) Then
        numbers.Append(c)
     ElseIf (Char.IsLetter(c)) Then
        characters.Append(c)
     End If
  Next

'Do whatever you have to do here


Angular Broadcast Asynchronous Events Okay I have the following use case:

Module A broadcasts an event.
Module B listens to that event.
Maybe also Module C listens to that event.

Now Module B has to perform an asynchronous operation before Module A may proceed with its code.

I solved this by creating an custom module $deferredEvents which allows to queue up asynchronous tasks before proceeding.

Example: http://jsfiddle.net/68845rw4/

Module:

angular.module('deferredEvents', []).provider('$deferredEvent', function () {
  this.$get = function ($q) {
    /**
     *  @param type String - 'broadcast' | 'emit'
     *  @param $scope Scope
     *  @param eventName String - name of the event
     */
    return function $deferredEvent(type, $scope, eventName) {
      var queueHelper = {
        queue: [],
        promise: $q.when(),
        /**
         * Promise will be executed after all previous promisses passed
         */
        appendThen: function appendThen () {
          return this.queuePromise(this.promise.then.apply(this.promise, arguments));
        },
        /**
         * Promise will be executed immediately
         */
        queuePromise: function queuePromise (promise) {
          if (promise === this.promise) {
            throw new Error('Can\'t wait for own promise');
          }
          // Execute promise
          promise = $q.when(promise);
          // Add the promise result to the queue
          this.promise = this.promise.then(function () {
            return promise;
          });
          return this.promise;
        }
      };
      var args = Array.prototype.slice.call(arguments, 3);
      var emitArgs = [eventName, queueHelper].concat(args);
      var event = $scope['$' + type].apply($scope, emitArgs);
      return queueHelper.promise.then(function() {
          return $q.all(queueHelper.queue);
        }).then(function (results) {
          var failed = results.some(function (val) {
            return val === false;
          });
          return failed ? $q.reject(event) : event;
        }, function () {
          return $q.reject(event);
        });
    };
  };
}).provider('$deferredEmit', function () {
  this.$get = function ($deferredEvent) {
    return function $deferredEmit($scope, eventName) {
      return $deferredEvent.apply(this, ['emit', $scope, eventName].concat(Array.prototype.slice.call(arguments, 2)));
    };
  };
}).provider('$deferredBroadcast', function () {
  this.$get = function ($deferredEvent) {
    return function $deferredBroadcast($scope, eventName) {
      return $deferredEvent.apply(this, ['broadcast', $scope, eventName].concat(Array.prototype.slice.call(arguments, 2)));
    };
  };
});

Idea seems nice but for me it's quite hard to follow. Don't get me wrong, it might just be me!

But could you achieve the same functionality A proceeds when B has done it's thing just by passing a callback function in your broadcasted/emitted event?

angular.module('moduleA', ['moduleB'])
  .controller('OneCtrl', function($scope, $rootScope) {
    // broadcast event
    $scope.send = function() {
      console.log('broadcast event');
      $rootScope.$broadcast('event:my-event', callback);
    };

    // event callback
    var callback = function(result) {
      console.log('callback', result);
    };
  });

angular.module('moduleB',[])
  .controller('TheOtherCtrl', function($scope, $q, $timeout) {
    // receive event
    $scope.$on('event:my-event', function(event, callback) {
      console.log('received', event.name);
      // execute callback with process result
      process(true).then(callback, callback);
    });

    // some async processing
    function process(success) {
      return  $timeout(function() {
        return success ? $q.when(true) : $q.reject(false);
      }, 800);
    }
  });

  broadcast event
  received event:my-event
  callback true


Colors changing in SDL program I have black and white SDL_Surface and I'm trying to change colors, such as from black to white and from white to red.  I did it, but it is taking too long to work.

SDL_Surface* tempSurface = SDL_DisplayFormat(textSurface);
for(int i=0;i<256;i++){
    SDL_SetColorKey(textSurface,SDL_SRCCOLORKEY,SDL_MapRGB(textSurface->format,i,i,i));
    SDL_FillRect(tempSurface, 0, SDL_MapRGB(tempSurface->format, 255, i, i));
    SDL_BlitSurface(textSurface,0,tempSurface,0);
    SDL_FreeSurface(textSurface);
    textSurface=tempSurface;
    tempSurface = SDL_DisplayFormat(textSurface);
}
SDL_FreeSurface(tempSurface);

It works for a few seconds, but it needs to run very fast.  It is C code, but it is not important. SDL is compatible with both languages.

How can I optimize this algorithm? Or can you suggest another way?

It would be helpful to know how textSurface is created. There may be a performance win to be had by using, say, a hardware surface instead of a software surface.

If you are using a surface that supports double buffering I would reccomend using SDL_FLip rather than flipping the buffers yourself using 2 surfaces.

E.g:

SDL_Surface* textSurface;
if(!(textSurface = SDL_SetVideoMode(600, 400, 32, SDL_HWSURFACE)))
    SDL_Quit();

SDL_SetColorKey(textSurface, SDL_SRCCOLORKEY, SDL_MapRGB(textSurface->format, 255, 0, 255)); //typically we use magenta
for(auto i=0; i<256; i++){
    auto col = SDL_MapRGB(textSurface->format, i, i, i);
    std::cout << col << "," << SDL_FillRect(textSurface, 0, col) << std::endl;
    SDL_Flip(textSurface);
}

Note that for your current goal, there is no need to reset the ColorKey each time around the loop.

If you want to run faster than "a few seconds" you certainly won't need to display all 256 intermediate colours to achieve the desired effect.

SDL_Surface* textSurface;
if(!(textSurface = SDL_SetVideoMode(600, 400, 32, SDL_HWSURFACE)))
    SDL_Quit();

SDL_SetColorKey(textSurface, SDL_SRCCOLORKEY, SDL_MapRGB(textSurface->format, 255, 0, 255)); //typically we use magenta
auto frame_increment = 4;
for(auto i=0; i<256; i += frame_increment){
    auto col = SDL_MapRGB(textSurface->format, i, i, i);
    std::cout << col << "," << SDL_FillRect(textSurface, 0, col) << std::endl;
    SDL_Flip(textSurface);
}

You should determine frame_increment based on your desired time and hardware capabilities.


Is this is good way to check for null? I ran into the code below today, and my gut instinct tells me this is an expensive way to do a null check. The point the author was making was that if you changed the name of the object then you don't need to change the value of the string being thrown in the exception.

The proposed use would be:

string cantBeNull=...;
Guard.IsNotNull(cantBeNull);
//instead of
if(cantBeNull == null) throw new ArgumentNullException("cantBeNull");

So Is this an acceptable way of checking for null? Is this overly expensive just to save you from not having the change the value passed to the exception?

Code in question:

public static void IsNotNull<T>(Expression<Func<T>> expression) where T : class
{
    if (expression == null)
        throw new ArgumentNullException("expression");

    var value = expression.Compile()();
    var param = (MemberExpression)expression.Body;
    var paramName = param.Member.Name;

    if (value != null)
        return;

    throw new ArgumentNullException(paramName);
}

How often do you change parameter names? How often does the code run?  Under any normal distribution of those, you should go with a minor code efficiency gain over a programmer efficiency gain.

On the other hand, if you (for some bizarre reason) change the parameter name almost every time the code runs, the tradeoff is probably worth it.

But just remember, that whether or not the string matches the actual parameter name, it should still let you find exactly which parameter was null so long as there isn't anywhere else in that function which throws an ArugmentNullException with the same string.


Symlinking shared libraries from a platform-specific directory We have a setup script for setting up a development environment, and setting up the image libraries is getting a bit messy:

# this is to get PNG and JPEG support working in PIL on Ubuntu
if [ -d /usr/lib/x86_64-linux-gnu ] ; then # 64 bit ubuntu
    sudo ln -s /usr/lib/x86_64-linux-gnu/libfreetype.so /usr/lib/
    sudo ln -s /usr/lib/x86_64-linux-gnu/libz.so /usr/lib/
    sudo ln -s /usr/lib/x86_64-linux-gnu/libjpeg.so /usr/lib/
fi

if [ -d /usr/lib/i386-linux-gnu/ ] ; then # 32 bit ubuntu
    sudo ln -s /usr/lib/i386-linux-gnu/libfreetype.so /usr/lib/
    sudo ln -s /usr/lib/i386-linux-gnu/libz.so /usr/lib/
    sudo ln -s /usr/lib/i386-linux-gnu/libjpeg.so /usr/lib/
fi

if [ -d /usr/lib64/ ] ; then # Redhat
    sudo ln -s /usr/lib64/libjpeg.so /usr/lib/
    sudo ln -s /usr/lib64/libz.so /usr/lib/
    sudo ln -s /usr/lib64/libfreetype.so /usr/lib/
fi

Seems like you can do the if statement to set a variable to the first part of the path, then put all the file copies at the end after that.

(Warning: My bash is kind of bad. Haven't used it for a while. I think this is how it's done though... Might need quotes around the libpath assignments?)

# this is to get PNG and JPEG support working in PIL on Ubuntu
if [ -d /usr/lib/x86_64-linux-gnu ] ; then # 64 bit ubuntu
    libpath=/usr/lib/x86_64-linux-gnu
fi

if [ -d /usr/lib/i386-linux-gnu/ ] ; then # 32 bit ubuntu
    libpath=/usr/lib/i386-linux-gnu
fi

if [ -d /usr/lib64/ ] ; then # Redhat
    libpath=/usr/lib64
fi

sudo ln -s $libpath/libfreetype.so /usr/lib/
sudo ln -s $libpath/libz.so /usr/lib/
sudo ln -s $libpath/libjpeg.so /usr/lib/


Read a file as 4-line chunks I want to extract the file content into chunks with perl. I have prepared some code snippets which read a file content into chunks.
I want to know what is the best way in Perl?

Example-File-Content:

1111
2222
3333
4444
5555
6666
7777
8888
9999
1212
1313
1414
1515

Example1:

#!/usr/bin/env perl
use strict;
use warnings;

open my $fh, '<', 'DUMP' or die $!;

until(grep !defined, my @chunk = map scalar <$fh>, 1 .. 4) {
    print @chunk, "\n";
}

Output:

1111
2222
3333
4444

5555
6666
7777
8888

9999
1212
1313
1414

Example2:

#!/usr/bin/perl -w
use strict;

open my $fh, '<', 'DUMP' or die $!;

my $i;
my @details;

while(<$fh>) {
    push @details, $_;
    next if ++$i % 4;
    print @details, "\n";
    @details = ();
}

Output:

1111
2222
3333
4444

5555
6666
7777
8888

9999
1212
1313
1414

Example3:

#!/usr/bin/perl -w
use strict;

open my $fh, '<', 'details.txt' or die $!;

sub read_lines {
    my ($fh, $amnt) = @_;
    return unless defined(my $line = <$fh>);
    return (
        $line,
        $amnt > 1 ? read_lines($fh, $amnt - 1) : ()
    );
}

while(my @details = read_lines($fh, 4)) {
    print @details, "\n";
}

Output:

1111
2222
3333
4444

5555
6666
7777
8888

9999
1212
1313
1414

1515

IMHO, the best way is the second example, but with some improvements:

#!/usr/bin/perl
use strict;
use warnings;

open my $fh, '<', 'DUMP' or die $!;
while(<$fh>) {
    print $_;
    print "\n" unless $. % 4;
}

There're no needs to temporary store in an array, and use the $. variable instead of a counter.

$. contains the current line number for the last filehandle accessed, see perlvar

If you want a separte function that returns the chunck:

#!/usr/bin/perl
use strict;
use warnings;

sub getChunck {
    my $file = shift;
    my @chunck;
    open my $fh, '<', $file or die "Unable to open '$file': $!";
    while(<$fh>) {
        push @chunck, $_;
        push @chunck, "\n" unless $. % 4;
    }
    return @chunck;
}

my @chunck = getChunck('DUMP');


Tiny function to write array or string to a file for testing with MVC, Ajax, etc This might not even warrant posting here, but I"m enjoying it and thought I would share.

//For Testing
function write_to_file($message){
        $handle = fopen("/path/to/logs/a_log_file.php", "a+");
        if (is_array($message)):
            fwrite($handle, "\nMessage is array.\t ");
            fclose($handle);
            file_put_contents('/path/to/logs/a_log_file.php', print_r($message, true),
                              FILE_APPEND | LOCK_EX);
        else:
            fwrite($handle, "\nMessage:\t " . $message);
            fclose($handle);
        endif;

        }

Haven't actually tested with multidimensional array yet and might be a good opportunity for some recursion.

Comments, suggestions and criticism are of course welcome.

Mixing fwrite and file_put_contents usage is a little strange, as they are functions which overlap in functionality. file_put_contents in fact is just a wrapper to call fopen, fwrite and fclose - expanding that call to it's component parts would therefore make what the code is doing more obviously repetitive.

So, the first goal I would suggest is to avoid repeatedly opening the file to append text to it; the goal of the function is simply to append bytes to a file - so make the function do that, in one write call. An example of what I mean is:

function write_to_file($message)
{
    $header = "\nMessage:\t ";

    if (is_array($message)) {
        $header = "\nMessage is array.\n";
        $message = print_r($message, true);
    }

    file_put_contents(
        '/path/to/logs/a_log_file.php',
        $header . $message,
        FILE_APPEND | LOCK_EX
    );
}


Dynamic Array implementation in Python I am trying to learn data structures by taking this Udemy Course, Easy to Advanced Data Structures by William Fiset. The course provided an implementation of Dynamic Array in Java. To ensure I fully understood the material, I tried to re-implement it in Python. Have provided the code below. Please let me know if I am doing it right or if there is anything wrong with it. Much thanks for your help!

"""
A generic dynamic array implementation

"""
class DynamicArray:
    def __init__(self, capacity=0):
        self._index = 0
        self.capacity = capacity # actual array size
        self.arr = [None for _ in range(self.capacity)]
        self.size = 0 # length user thinks array is

    def __len__(self):
        return self.size

    def isEmpty(self):
        return self.size == 0

    def __getitem__(self, index):
        return self.arr[index]

    def __setitem__(self, index, elem):
        self.arr[index] = elem

    def clear(self):
        for i in range(self.size): self.arr[i] = None

    def add(self, elem):
        # To resize
        if self.size + 1 >= self.capacity:
            if self.capacity == 0:
                self.capacity = 1
            else:
                self.capacity *= 2
            new_arr = DynamicArray(self.capacity)
            for i in range(self.size):
                new_arr[i] = self.arr[i]
            self.arr = new_arr
        self.arr[self.size] = elem
        self.size += 1

    # Removes an element at the specified index in this array
    def removeAt(self, rm_index):
        if rm_index >= self.size or rm_index < 0:
            raise IndexError
        data = self.arr[rm_index]
        new_arr = DynamicArray(self.size - 1)
        i, j = 0, 0
        while i < self.size: #self.size = 3
            if i == rm_index:
                j -= 1
            else:
                new_arr[j] = self.arr[i]
            i += 1
            j += 1
        self.arr = new_arr
        self.size -= 1
        return data

    def remove(self, elem):
        index = self.indexOf(elem)
        if index == -1: return False
        self.removeAt(index)
        return True

    def indexOf(self, elem):
        for i in range(self.size):
            if elem == self.arr[i]:
                return i
        return -1

    def __contains__(self, elem):
        return self.indexOf(elem) != -1

    def __iter__(self):
        return self

    def __next__(self):
        if self._index > self.size: raise StopIteration
        else:
            data = self.arr[self._index]
            self._index += 1
            return data

    def __str__(self):
        if self.size == 0: return "[]"
        else:
            ans = "["
            for i in range(self.size - 1):
                ans += str(self.arr[i]) + ", "
            ans += str(self.arr[self.size - 1]) + "]"
        return ans

Your code looks good, but there are four things I would improve:

Style

Overall your code follows the PEP 8 Style Guide, but:

Names should use snake_case, so index_of instead of indexOf, etc
Comments after code should leave 2 white spaces after the code:

self.size = 0 # length user thinks array is  <- wrong
self.size = 0  # length user thinks array is <- correct

I don't know if this is just my preference, but I think it's better to group the public methods like is_empty, index_of etc and group the overloads like __getitem__, __setitem__

Clear

At least for me, what I would expect of a method called clear is that it removes all objects, leaving the array empty. So in my opinion your clear method should just set self.size = 0. You don't need to set the elements to null because they don't matter anymore.

Is empty?

In Python, you can check if a list contains any elements by doing:

if my_list:

I think users would expect the same behaviour for your class, which you can implement with the __bool__ (Python 3.x) or __nonzero__ (Python 2.x) methods. Just return not is_empty()

Iterator

The biggest flaw I see in the code is your implementation of iteration. You are keeping the index in the array object; this means that the user cannot do:

for x in my_array:
    for y in my_array:

Because the _index is shared in both loops.

You can solve this by implementing the iterator in a different class. I would declare it as a nested class, starting with an underscore to indicate the user that it should be considered private:

class DynamicArray:
    class _Iterator:
        def __init__(self, dynamic_array):
             # ....
        # Implement '_index' and '__next__' in this class

    def __iter__(self):
        # Return a different object every time you are requested an iterator
        return _Iterator(self)


Profit sensitivity analysis in base R Using base R, I've conducted a simple profit sensitivity analysis, i.e. measuring the impact on profit if there is a change in price, variable cost per sale, unit sales or fixed costs assuming that there are no interaction effects between the dependent variables, e.g. a change in price does not change unit sales.

I'd like to have feedback on my code, which is 'just for fun'. Particularly I'm interested in:

Making the loops slicker
Assigning the matrices full of zeroes with fewer bytes if I can

# Aim: conduct a profit sensitivity analysis

# Define parameters

Param   = c(10,5,100000,10000) # I.e. a business with a price, variable cost, unit sales and fixed cost of these values

Max_Change  = 0.1
Changes     = seq(-Max_Change, Max_Change, by=0.01)

# Define profit function

Profit  = function(Price, Var_Cost, Units, Fixed_Cost){
Profit  = (Price - Var_Cost)*Units - Fixed_Cost
return(Profit)
}

# Calculating base and updated profit

Start_Profit = do.call("Profit", as.list(Param))

New_Price       = matrix(0,length(Changes),4)
New_Var_Cost    = matrix(0,length(Changes),4)
New_Unit_Sales  = matrix(0,length(Changes),4)
New_Fixed_Costs     = matrix(0,length(Changes),4)

for(i in 1:length(Changes))
{
New_Price[i,]       = c((1+Changes[i])*Param[1], Param[2], Param[3], Param[4])
New_Var_Cost[i,]        = c(Param[1], (1+Changes[i])*Param[2], Param[3], Param[4])
New_Unit_Sales[i,]  = c(Param[1], Param[2], (1+Changes[i])*Param[3], Param[4])
New_Fixed_Costs[i,]     = c(Param[1], Param[2], Param[3], (1+Changes[i])*Param[4])
}

Profit_New_Price        = matrix(0,length(Changes),1)
Profit_New_Var_Cost     = matrix(0,length(Changes),1)
Profit_New_Unit_Sales   = matrix(0,length(Changes),1)
Profit_New_Fixed_Costs  = matrix(0,length(Changes),1)

for(i in 1:length(Changes))
{
Profit_New_Price[i,]        = do.call("Profit", as.list(New_Price[i,]))
Profit_New_Var_Cost[i,]     = do.call("Profit", as.list(New_Var_Cost[i,]))
Profit_New_Unit_Sales[i,]   = do.call("Profit", as.list(New_Unit_Sales[i,]))
Profit_New_Fixed_Costs[i,]  = do.call("Profit", as.list(New_Fixed_Costs[i,]))
}

Diff_Profit_New_Price       = Profit_New_Price-Start_Profit
Diff_Profit_New_Var_Cost    = Profit_New_Var_Cost-Start_Profit
Diff_Profit_New_Unit_Sales  = Profit_New_Unit_Sales-Start_Profit
Diff_Profit_New_Fixed_Costs     = Profit_New_Fixed_Costs-Start_Profit

# Plot the profit sensitivities

options(scipen=999) # Remove scientific notation from the chart axes

msg=paste("Starting Business Conditions: Price = £",
    format(Param[1],big.mark=",",scientific=FALSE),
    ", Variable Cost = £",
    format(Param[2],big.mark=",",scientific=FALSE),
    ", Unit Sales = ",
    format(Param[3],big.mark=",",scientific=FALSE),
    ", Fixed Costs = £",
    format(Param[4],big.mark=",",scientific=FALSE),
    ", Giving a Profit of £",
    format(Start_Profit,big.mark=",",scientific=FALSE))

plot(100*Changes,Diff_Profit_New_Price,
    xlab="Change in Dependent Variable (%)",
    ylab="Change in Profit (£)",
    main="Profit Sensitivity Analysis of Changing One Variable at a Time, Assuming no Interaction Among Dependent Variables",
    type="l",
    sub=msg)

lines(100*Changes, Diff_Profit_New_Var_Cost, col="red")
lines(100*Changes, Diff_Profit_New_Unit_Sales, col="blue")
lines(100*Changes, Diff_Profit_New_Fixed_Costs, col="green")

# Building legend

Leg_Names = c("Price Change", "Variable Cost Change", "Unit Sales Change", "Fixed Costs Change")
legend("bottomright",
    legend=Leg_Names,
    col=c("black","red","blue","green"),
    pch=15,
    bty="n")

The main thing to note is that your profit function only uses vectorized functions (- and *) so it is vectorized with respect to its four inputs. This means that you do not have to create loops; instead you can just feed the functions with vector(s): one vector for the variable that you are shocking, and scalars for the other three fixed inputs. Here is my suggested rewrite, having put everything into a function:

biz_sens_analysis <- function(Price      = 10,
                              Var_Cost   = 5,
                              Units      = 100000,
                              Fixed_Cost = 10000) {

  Max_Change <- 0.1
  Changes <- seq(-Max_Change, Max_Change, by = 0.01)
  shock <- function(x) x * (1 + Changes)

  # Define profit function
  Profit <- function(Price, Var_Cost, Units, Fixed_Cost)
     (Price - Var_Cost) * Units - Fixed_Cost

  # Calculating base and updated profit
  Start_Profit <- Profit(Price, Var_Cost, Units, Fixed_Cost)

  Profit_New_Price       <- Profit(shock(Price), Var_Cost, Units, Fixed_Cost)
  Profit_New_Var_Cost    <- Profit(Price, shock(Var_Cost), Units, Fixed_Cost)
  Profit_New_Unit_Sales  <- Profit(Price, Var_Cost, shock(Units), Fixed_Cost)
  Profit_New_Fixed_Costs <- Profit(Price, Var_Cost, Units, shock(Fixed_Cost))

  diff_px <- function(px) px - Start_Profit
  Diff_Profit_New_Price       <- diff_px(Profit_New_Price)
  Diff_Profit_New_Var_Cost    <- diff_px(Profit_New_Var_Cost)
  Diff_Profit_New_Unit_Sales  <- diff_px(Profit_New_Unit_Sales)
  Diff_Profit_New_Fixed_Costs <- diff_px(Profit_New_Fixed_Costs)

  # Plot the profit sensitivities
  options(scipen = 999) # Remove scientific notation from the chart axes

  fmt <- function(x) format(x, big.mark=",", scientific = FALSE)
  msg <- paste("Starting Business Conditions: Price = £", fmt(Price),
             ", Variable Cost = £",    fmt(Var_Cost),
             ", Unit Sales = ",        fmt(Units),
             ", Fixed Costs = £",      fmt(Fixed_Cost),
             ", Giving a Profit of £", fmt(Start_Profit))

  plot(100 * Changes, Diff_Profit_New_Price,
     xlab = "Change in Dependent Variable (%)",
     ylab = "Change in Profit (£)",
     main = "Profit Sensitivity Analysis of Changing One Variable at a Time, Assuming no Interaction Among Dependent Variables",
     type = "l",
     sub  = msg)

  lines(100 * Changes, Diff_Profit_New_Var_Cost,    col = "red")
  lines(100 * Changes, Diff_Profit_New_Unit_Sales,  col = "blue")
  lines(100 * Changes, Diff_Profit_New_Fixed_Costs, col = "green")

  # Building legend
  Leg_Names <- c("Price Change", "Variable Cost Change", "Unit Sales Change", "Fixed Costs Change")
  legend("bottomright",
         legend = Leg_Names,
         col    = c("black", "red", "blue", "green"),
         pch    = 15,
         bty    = "n")
}

biz_sens_analysis()

Hope it helps. Let me know if you have questions.


Lookup country codes with a JavaScript ES6 Map Object In my application, a country code may be passed from the client as a URL query string or in a header. If there is no country in the URL or headers, then it will default to the United States. The application uses both Alpha-2 and Alpha-3 codes, and with the header there are special cases for Canada and Great Britain.

const countries = {
  UNITED_STATES: ['US', 'USA'],
  UNITED_KINGDOM: ['UK', 'GBR'],
  CANADA: ['CA', 'CAN'],
  AUSTRALIA: ['AU', 'AUS'],
  PUERTO_RICO: ['PR', 'PRI'],
  GREAT_BRITAIN: ['GB', 'GBR']
};

function lookupCountry(obj, country) {
  return Object.keys(obj).map((el) => obj[el])
         .filter((countryArr) => countryArr.includes(country.toUpperCase()));
}

function getCountry(i18nHeader, i18nQuery) {
  if (i18nQuery) {
    return lookupCountry(countries, i18nQuery);
  } else {
    if (!i18nHeader || !i18nHeader.length) {
      return countries.UNITED_STATES;
    } else {
      if (i18nHeader.toUpperCase() === countries.GREAT_BRITAIN[0]) {
        return countries.UNITED_KINGDOM;
      }
      if (i18nHeader.toUpperCase() === countries.CANADA[0]) {
        return countries.UNITED_STATES;
      }
    }
    return lookupCountry(countries, i18nHeader);
  }
}

I would like to take advantage of new ES6 language features and use the Map object. I'm thinking this Map structure could be initialized and structured like this:

 const countries = new Map([
      ['UNITED_STATES', ['US', 'USA']],
      [ 'GREAT_BRITAIN', ['GB', 'UK']]...
  ]);

I could then use counteries.get('GREAT_BRITAIN') to retrieve the array of country codes.

The natural data structure for looking up things is a map. In JavaScript this is simply an object and is written like {key: value}. It provides efficient access.

Therefore my first idea was to use two maps for the two kinds of lookup you need: country by ISO-3166-alpha2 and country by ISO-3166-alpha3. So instead of your single countries map, I prefer to have these two maps:

const countryByIso2 = {};
const countryByIso3 = {};

These are currently empty but will be filled soon.

function init() {
    const countryCodes = [
        ['US', 'USA'],
        ['UK', 'GBR'],
        ['CA', 'CAN'],
        ['AU', 'AUS'],
        ['PR', 'PRI'],
        ['GB', 'GBR']
    ];

    const overrides = [
        ['GB', 'UK'],
        ['CA', 'US']
    ];

    for (let country of countryCodes) {
        countryByIso2[country[0]] = country;
        countryByIso3[country[1]] = country;
    }
    for (let override of overrides) {
        let from = countryByIso2[override[0]];
        let to = countryByIso2[override[1]];
        countryByIso2[from[0]] = countryByIso2[to[0]];
        countryByIso3[from[1]] = countryByIso3[to[1]];
    }
}

init();

In this function I have removed all the unnecessary data and left just the country codes. There are two configuration sections. The first is the mapping you already know. The second are the overrides that you wrote in the if clauses of your getCountry function.

After the first part of the initialization, countryByIso2['CA'] contains ['CA', 'CAN']. But when the overrides are done, they overwrite the existing map entries. This means that after the initialization you can inspect the countryByIso2 object and quickly see what you will get.

What's missing is the actual lookup.

function lookupCountry(code) {
    if (!code) {
        return null;
    }
    let upperCode = code.toUpperCase();
    return countryByIso2[upperCode]
        || countryByIso3[upperCode];
}

function getCountry(i18nHeader, i18nQuery) {
    return lookupCountry(i18nHeader)
        || lookupCountry(i18nQuery)
        || countryByIso2['US'];
}

In total, this code is longer than yours, but it uses only very few and very fast operations. It is suitable for country lists of several thousand. Your code on the other hand walks through all entries (lookupCountry) and takes a long time when the data grows.

There's another difference. If i18nHeader is set to some unknown value, your code returns undefined while my code returns the US country codes.

The next topic is putting your code into its own namespace. After the above definitions, all parts of the code (even those you didn't write) can access the init function, the countryByIso2 tables, the lookupCountry function. But these are meant to be internal to your code and should not be seen by any unrelated code.

Therefore, it is best practice to explicitly define those pieces of code you want to export to the other code and only make that visible. This is typically done with the following pattern, putting all your code into an anonymous function and then returning the things to be exported:

const getCountry = (function() {

    const countryByIso2 = {};
    const countryByIso3 = {};

    const countryCodes = [
        ['US', 'USA'],
        ['UK', 'GBR'],
        ['CA', 'CAN'],
        ['AU', 'AUS'],
        ['PR', 'PRI'],
        ['GB', 'GBR']
    ];

    const overrides = [
        ['GB', 'UK'],
        ['CA', 'US']
    ];

    for (let country of countryCodes) {
        countryByIso2[country[0]] = country;
        countryByIso3[country[1]] = country;
    }
    for (let override of overrides) {
        let from = countryByIso2[override[0]];
        let to = countryByIso2[override[1]];
        countryByIso2[from[0]] = countryByIso2[to[0]];
        countryByIso3[from[1]] = countryByIso3[to[1]];
    }

    function lookupCountry(code) {
        if (!code) {
            return null;
        }
        let upperCode = code.toUpperCase();
        return countryByIso2[upperCode]
            || countryByIso3[upperCode];
    }

    function getCountry(i18nHeader, i18nQuery) {
        return lookupCountry(i18nHeader)
            || lookupCountry(i18nQuery)
            || countryByIso2['US'];
    }

    return getCountry;
})();

When I saw your code, the first thing I did was to remove the unnecessary else clauses from the getCountry function. Then it looked like this:

function getCountry(i18nHeader, i18nQuery) {
    if (i18nQuery) {
        return lookupCountry(countries, i18nQuery);
    }
    if (!i18nHeader || !i18nHeader.length) {
        return countries.UNITED_STATES;
    }
    if (i18nHeader.toUpperCase() === countries.GREAT_BRITAIN[0]) {
        return countries.UNITED_KINGDOM;
    }
    if (i18nHeader.toUpperCase() === countries.CANADA[0]) {
        return countries.UNITED_STATES;
    }
    return lookupCountry(countries, i18nHeader);
}

It's a one-dimensional chain of decisions now, instead of a hard-to-grasp nested decision tree. It's still difficult to follow, and that's why I rewrote it in the alternative way described above.


Thread pool using boost::thread I would like to make a portable thread pool using boost::thread. Some one could tell me to look at boost::thread_group and boost::asio. I know, but I am not really familiar with these things so I don't know if they are suitable for my purpose.

Please give me comments on what I supposed to do here.

JobItem and JobQueue:

class JobItem
{
private:
    uint32_t        job_type_;      //!< Job type
    char*           job_ptr_;       //!< Pointer to job content
// ...
};

class JobQueue
{
private:
    queue<JobItem*>                 job_queue_;
public:
    // Definitely I need to use mutex and condition variable here
    JobItem* pick();
    void push(JobItem* job_item);
};

JobWorker (worker thread):

class JobWorker
{
private:
    JobQueue* shared_job_queue_;
    boost::thread the_thread_;

public:
    JobWorker(JobQueue* shared_queue) : shared_job_queue_(shared_queue);

    // exec_job will be overridden by concrete subclasses
    virtual void exec_job(JobItem* job_item) = 0;
    void run()
    {
        while(true)
        {
            JobItem* job = shared_job_queue_->pick();
            exec_job(job);
            delete job;
        }
    }
    void start()
    {
        the_thread_ = boost::thread(boost::bind(&JobWorker::run, this));
    }
    void stop()
    {
        the_thread_.join();
    }
};

Concrete JobWorker subclasses:

class Task_1_Worker : public JobWorker
{
    // override exec_job
    void exec_job(JobItem* job)
    {
        // Process job item here
    }
}

class Task_2_Worker : public JobWorker
{
    // override exec_job
    void exec_job(JobItem* job)
    {
        // Process job item here
    }
}

Now, WorkerPool with template:

template <class JobWorkerT>
class WorkerPool
{
private:
    JobQueue shared_job_queue_;
    list<JobWorkerT> job_workers_;

public:
    WorkerPool(const int num_threads);
    ~WorkerPool(void) {};

    void push(JobItem* job)
    {
        shared_job_queue_.push(job);
    }
    void start();
    void stop();
};

template <class JobWorkerT>
WorkerPool<JobWorkerT>::WorkerPool(const int num_threads)
{
    for(int i=0; i<num_threads; i++)
    {
        JobWorkerT job_worker(&shared_job_queue_);
        job_workers_.push_back(job_worker);
    }
}

template <class JobWorkerT>
void WorkerPool<JobWorkerT>::start()
{
    for(std::list<JobWorkerT>::iterator iter = job_workers_.begin(); iter != job_workers_.end(); iter++)
    {
        ((JobWorkerT)(*iter)).start();
    }
}

template <class JobWorkerT>
void WorkerPool<JobWorkerT>::stop()
{
    for(std::list<JobWorkerT>::iterator iter = job_workers_.begin(); iter != job_workers_.end(); iter++)
    {
        ((JobWorkerT)(*iter)).stop();
    }
}

main()

int main()
{
    WorkerPool<Task_1_Worker>* task1_grp = new WorkerPool<Task_1_Worker>();
    WorkerPool<Task_2_Worker>* task2_grp = new WorkerPool<Task_2_Worker>();

    task1_grp->start();
    task2_grp->start();

    // Producers should call task1_grp.push(JobItem*) or task2_grp.push(JobItem*) to dispatch new jobs to thread pools
}

Design

Not sure why you have a virtual function on the worker?

virtual void exec_job(JobItem* job_item) = 0;

The worker is simple he picks up jobs and then does them. What the job is defines what work should be done. So I would have put the virtual function that defines the work on the JobItem.

The run() function of the worker is then simply.

void run()
{
    // You want some way for the thread to eventually exit.
    // You can make that happen by letting the queue return
    // a null object when the object is being shut down.
    JobItem* job;
    while(job = shared_job_queue_->pick())
    {
        job->do();
        delete job;
    }
}

Code Review

Prefer not to use pointers.

Pointers don't have the concept of ownership semantics. So we don't know who is responsible for deleting them. Use a type that conveys ownership semantics so the person reading the code understands the semantics.

 JobQueue* shared_job_queue_;

You always need a job queue. So this can never be nullptr. So rather than a pointer a better choice would have been a reference. Also a reference implies there is no ownership of the object so you are not supposed to delete it.

class JobWorker
{
private:
    JobQueue& shared_job_queue_;
public:
    JobWorker(JobQueue& shared_queue)
        : shared_job_queue_(shared_queue)
    {}

Again in the JobQueue. You push and pop pointers. So there is no concept of ownership.

class JobQueue
{
private:
    queue<JobItem*>                 job_queue_;
public:
    // Definitely I need to use mutex and condition variable here
    JobItem* pick();
    void push(JobItem* job_item);
};

As a user of this class I could easily accidently pass the address of a Job object that is actual an automatic variable. Unless I read the code it is not obvious that the code will blow up. Even worse the code may not blow up for a while since the pointer is not deleted until after the work has been done.

In this case I would use a smart pointer. The obvious one is std::unique_ptr. When you use this as a parameter you are indicating that you are accepting ownership of the object.

class JobQueue
{
private:
    // You are maintaining a queue of owned pointers.
    // You will take responsibility for deleting them when this
    // object is destroyed.
    queue<std::unique_ptr<JobItem>>      job_queue_;
public:
    // This method returns a pointer.
    // More importantly it is returning ownership of the pointer to
    // you uniquely so you are the sole owner of the pointer and the
    // JobQueue is no longer retaining any pointers to this object.
    std::unique_ptr<JobItem> pick();

    // This method accepts a unique_ptr.
    // Because it accepts the unique_ptr by value you are loosing
    // ownership. Once this method is called you will no longer own
    // the object it will be gone from the object you passed.
    void push(std::unique_ptr<JobItem> job_item);
};

Use modern C++

Its 2016. C++11 has been out for 5 years. C++14 has been out for two years. Both have std::thread and the associated parallel libraries.


Deriving stars from a product score This function derives a 'stars' rating from a product score that is retrieved from local storage.  What would be the best way to refactor these else if statements?

    $scope.ratingStars = function(productID) {

        var bottomLine = $filter('filter')($localStorage.bottomlinesObject.bottomlines, {domain_key:''+productID})//jshint ignore:line

        if(bottomLine.length > 0) {

            if (bottomLine[0].product_score === 'undefined') {return 0;}//jshint ignore:line
            else if(bottomLine[0].product_score > 0 && bottomLine[0].product_score <=1.5)  {return 1;}// jshint ignore:line
            else if(bottomLine[0].product_score > 1.5 && bottomLine[0].product_score <=2) {return 2;}//jshint ignore:line
            else if(bottomLine[0].product_score > 2 && bottomLine[0].product_score <=2.5) {return 2.5;}//jshint ignore:line
            else if(bottomLine[0].product_score > 2.5 && bottomLine[0].product_score <=3.2) {return 3;}//jshint ignore:line
            else if(bottomLine[0].product_score > 3.2 && bottomLine[0].product_score <=3.7) {return 3.5;}//jshint ignore:line
            else if(bottomLine[0].product_score > 3.7 && bottomLine[0].product_score < 4.3) {return 4;}//jshint ignore:line
            else if(bottomLine[0].product_score >= 4.3 && bottomLine[0].product_score < 4.75) {return 4.5;}//jshint ignore:line
            else if(bottomLine[0].product_score >= 4.75 && bottomLine[0].product_score < 5) {return 5;}//jshint ignore:line
            else if(bottomLine[0].product_score >= 5) {return 5;}// jshint ignore:line

        }

        else {return 0;}

    };

Since the intervals aren't consistent, rounding wouldn't help, but you can drop the floor condition since you're using else-if.

    if(bottomLine.length > 0) {
        if (bottomLine[0].product_score === 'undefined') {return 0;}//jshint ignore:line
        else if(bottomLine[0].product_score <=1.5)  {return 1;}// jshint ignore:line
        else if(bottomLine[0].product_score <=2) {return 2;}//jshint ignore:line
        else if(bottomLine[0].product_score <=2.5) {return 2.5;}//jshint ignore:line
        else if(bottomLine[0].product_score <=3.2) {return 3;}//jshint ignore:line
        else if(bottomLine[0].product_score <=3.7) {return 3.5;}//jshint ignore:line
        else if(bottomLine[0].product_score < 4.3) {return 4;}//jshint ignore:line
        else if(bottomLine[0].product_score < 4.75) {return 4.5;}//jshint ignore:line
        else {return 5;}//jshint ignore:line
    }

And, since you're returning each time, you can drop the "else" part of the else if for that matter.

    if(bottomLine.length > 0) {
        if (bottomLine[0].product_score === 'undefined') {return 0;}//jshint ignore:line
        if(bottomLine[0].product_score <=1.5)  {return 1;}// jshint ignore:line
        if(bottomLine[0].product_score <=2) {return 2;}//jshint ignore:line
        if(bottomLine[0].product_score <=2.5) {return 2.5;}//jshint ignore:line
        if(bottomLine[0].product_score <=3.2) {return 3;}//jshint ignore:line
        if(bottomLine[0].product_score <=3.7) {return 3.5;}//jshint ignore:line
        if(bottomLine[0].product_score < 4.3) {return 4;}//jshint ignore:line
        if(bottomLine[0].product_score < 4.75) {return 4.5;}//jshint ignore:line
        return 5; //jshint ignore:line
    }

I think you should remove the redundant floor conditions, but I can see a case made either way with leaving the "else" in place.


Brute-Force algorithm in C++ I wrote a brute-force algorithm which shall find all possible combinations of ASCII-values that can sum up to a specific value (int hashval).

The algorithm is derived from a recursive algorithm that could print all binary numbers that are possible within a specific length characters. I modified it, so it now iterates over all ints between first and last, thus building all combinations there are. If the sum of the combination is equal to hashval, then the combination is saved in vector<vector<int[]>> combinations. When all possible combinations are found the content of combinations is printed into a file.

I already use some little tricks I learned for competitive programming, but the algorithm is still too slow. At the moment it is running for approximately 3 hours for no more than 13 characters, with no end in sight, file I/O hasn't even started yet. I know that its Big-O for runtime is horrible, no doubt. (if I get it right it is O(m^n) ).

I considered adding a check for n=1 since that would leave only one possibility for the last character, saving some recursive calls, but that would add another if to be called in every function call, either.

What more ways are there to improve the speed? Any improvements are not bound to C++, I will take a look at every programming language which might be more efficient in this case, though I guess there scarcely will be one.

The algorithm:

#include <iostream>
#include <vector>
#include <bits/stdc++.h>
#include <cstdio>

using namespace std;
using namespace std::chrono;

//values in decimal
int first = 32, last = 126, characters = 13, hashval = 1207;

vector<vector<int>> combinations;

void brute(int n, int it, vector<int> values) {
    if (n == 0) {
        int sum = 0;

        //sum up recent values
        for (int j = 0; j < characters; ++j) {
            sum += values[j];
        }
        //save values if matching to hash
        if (sum == hashval) {
            combinations.push_back(values);
        }
    } else {
        for (int i = first; i <= last; ++i) {
            values[it] = i;
            brute(n - 1, it + 1, values);
        }
    }
}

Printing the output to a file:

void printToFile(){
    freopen("fileContent.brute", "w", stdout);
    cout << "\n\nFound " << combinations.size() << " combinations." << endl;
    int counter = 0;
    for (auto combo = combinations.begin(); combo != combinations.end(); ++combo){
        for (int i = 0; i <  characters; ++i) {
            cout << (char) combinations.at(counter).at(i) << " ";
        }
        cout<< "\r\n";
        counter++;
    }
}

The main-method:

int main() {
    auto start = high_resolution_clock::now();
    //characters = 3, first = 0, last = 3, hashval = 6;
    ios_base::sync_with_stdio(false);
    cin.tie(nullptr);

    vector<int> values;

    for (int i = 0; i < characters; ++i) {
        values.push_back(first);
    }

    brute(characters, 0, values);

    printToFile();
    auto stop = high_resolution_clock::now();
    auto duration = duration_cast<seconds>(stop - start);
    cout << "Brute-forcing needed " << duration.count() << " seconds.\r\n";

    cout.flush();

    return 0;
}

Use a better container

The main issue is that std::vector<int> is a bad container for storing a set of ASCII characters. Every vector allocates on the heap, and an empty vector is likely 24 bytes in size. So this is huge waste. Why not use a container designed to hold a series of ASCII characters, like... std::string!

Do you need to store all possible combinations in memory?

You are first building a vector of all possible combinations with the desired sum. But the only thing you are doing with it afterwards is writing the result to a file. That's a huge waste of memory, and maintaining the vector of vectors in itself costs some CPU time as well. Why not write the results to a file immediately?

Exit early when your combination has already exceeded the desired sum

You always try to fill the vector values with characters elements, and try each possible element at each position. However, in brute(), you should calculate the partial sum of elements 0 up to n, and then just check in the for loop whether adding i to that sum would be equal to or higher than the desired sum. If so, if it's equal output the combination, but in any case return at that point instead of trying other values.

Use \n consistently to end lines

You are mixing \n, \r\n and std::endl in your code. Stick to \n, it is the standard way of ending a line in C and C++. The underlying I/O routines will automatically convert the newlines to \r\n on Windows.

std::endl is equivalent to \n plus a call to flush(). Unless there is a good reason to, you don't want this extra flushing to happen, since it reduces the performance of your program. Normally, std::cout is line-buffered, so if something ends with a \n, it is already flushed automatically. This also means you should remove the explicit call to cout.flush().


Handling float discrepancy in Unity3d I know there are thousands of posts and questions around the topic of Float Discrepancy. And this is not really a question but a discussion for it.

After sitting repeatedly in front of the same problem, I made the following class to handle it once and for all of our projects.  The direct value comparison is from the book The Art of Computer Programming by Donald E. Knuth. I just changed to unitys Mathf functionality.

So what is your take on the problem? Anything to add, something wrong?

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public static class FloatDiscrepancy {
  public static float Accuracy = 0.00001f;

  public static float SqrAccuracy {
    get {
     return Accuracy * Accuracy;
    }
    private set { }
  }

  public static bool V4Equal(Vector4 a, Vector4 b){
    return Vector4.SqrMagnitude(a - b) < SqrAccuracy;
  }

  public static bool V3Equal(Vector3 a, Vector3 b){
    return Vector3.SqrMagnitude(a - b) < SqrAccuracy;
  }

  public static bool V2Equal (Vector2 a, Vector2 b){
    return Vector2.SqrMagnitude (a - b) < SqrAccuracy;
  }

  public static bool QuaternionEqual(Quaternion a, Quaternion b){
    return Quaternion.Angle (a, b) < Accuracy;
  }

  //Took the following from literature: The Art of Computer Programming by Donald E. Knuth
  public static bool approximatelyEqual(float a, float b){
    return Mathf.Abs(a - b) <= ( (Mathf.Abs(a) < Mathf.Abs(b) ? Mathf.Abs(b) : Mathf.Abs(a)) * Accuracy);
  }

  public static bool essentiallyEqual(float a, float b){
    return Mathf.Abs(a - b) <= ( (Mathf.Abs(a) > Mathf.Abs(b) ? Mathf.Abs(b) : Mathf.Abs(a)) * Accuracy);
  }

  public static bool definitelyGreaterThan(float a, float b){
    return (a - b) > ( (Mathf.Abs(a) < Mathf.Abs(b) ? Mathf.Abs(b) : Mathf.Abs(a)) * Accuracy);
  }

  public static bool definitelyLessThan(float a, float b){
    return (b - a) > ( (Mathf.Abs(a) < Mathf.Abs(b) ? Mathf.Abs(b) : Mathf.Abs(a)) * Accuracy);
  }
}

I can't quite figure out in which situation you would use approximatelyEqual() and in which essentiallyEqual(). and what the difference really should be.

I made some tests and overall it seems to work. I did though find the below strange behavior:

Testing with these two values:

    float a = 1.0f;
    float b = 1.0f + 1e-6f;

    Console.WriteLine($"a: {a:F15}");
    Console.WriteLine($"b: {b:F15}");
    Console.WriteLine($"a - b: {a - b:F15}");
    Console.WriteLine($"b - a: {b - a:F15}");
    Console.WriteLine();

    Console.WriteLine($"approximatelyEqual: {approximatelyEqual(a, b)}");
    Console.WriteLine($"essentiallyEqual: {essentiallyEqual(a, b)}");
    Console.WriteLine($"definitelyGreaterThan: {definitelyGreaterThan(a, b)}");
    Console.WriteLine($"definitelyLessThan: {definitelyLessThan(a, b)}");
    Console.WriteLine();
    Console.WriteLine($"HEquals: {HEquals(a, b)}");
    Console.WriteLine($"HGreaterThan: {HGreaterThan(a, b)}");
    Console.WriteLine($"HLessThan: {HLessThan(a, b)}");

I get the expected output:

a: 1.000000000000000
b: 1.000001000000000
a - b: -0.000000953674300
b - a: 0.000000953674300

approximatelyEqual: True
essentiallyEqual: True
definitelyGreaterThan: False
definitelyLessThan: False

HEquals: True
HGreaterThan: False
HLessThan: False

But if I try this:

    float a = 0.0f;
    float b = 1e-6f;

I get this:

a: 0.000000000000000
b: 0.000001000000000
a - b: -0.000001000000000
b - a: 0.000001000000000

approximatelyEqual: False
essentiallyEqual: False
definitelyGreaterThan: False
definitelyLessThan: True

HEquals: True
HGreaterThan: False
HLessThan: False

But I would have expected the same result?

The H-functions is my own naive implementations:

public static bool HEquals(float a, float b)
{
  return Math.Abs(a - b) < Accuracy;
}

public static bool HGreaterThan(float a, float b)
{
  return a - b >= Accuracy;
}

public static bool HLessThan(float a, float b)
{
  return a - b <= -Accuracy;
}

Disclaimer: I'm not using Unity3ds Mathf.Abs() but System.Math.Abs()


Removing duplicates in an array Problem: Remove duplicates from a given array of integers.

I want this method to work without using data sets except an array.

My code is currently \$O(n^2 + n) = O(n^2)\$. How do I shorten this? My main goal in writing this code was in the mindset that I am in an interview and that I am given only 20 minutes to solve this problem. Thus, I tried to find the quickest way of solving the problem. Advice on how better to approach these problems would also be very helpful!

public int[] removeDuplicate(int[] arr)
{
    int temp[] = new int[arr.length];
    boolean[] check = new boolean[arr.length];
    int index = 0;
    for(int i = 0; i < arr.length; i++)
    {
        if(check[i] != true)
            temp[index++] = arr[i];
        else
            continue;

        int var = arr[i];
        for(int j = i+1; j < arr.length; j++)
        {
            if(var == arr[j])
                check[j] = true;
        }
    }
    int returnArray[] = new int[index];

    for(int i = 0; i < returnArray.length; i++)
        returnArray[i] = temp[i];

    return returnArray;
}

Problem: Remove duplicates from a given array of integers.

An important part of programming interviews is asking clarifying questions,
and try to not assume anything.
For example:

What kind of values are we talking about?
What is the range of possible values?
Is the array sorted?
How many values are there?
How many duplicate values are there?
Do you need to preserve the order of the elements that are not removed?

Depending on the answers, you may have a very different problem to solve.
For example if the range of values is limited to [1:100], or if the array is sorted.

My gut reaction is to assume that the values are in the full range of valid integers, the array is not sorted, and there may be any number of duplicate values. However, during an interview it's important to not assume anything. Whether the candidate asks clarifying questions or just quietly makes assumptions can make a big difference.
(How many times do programmers think they know what system they are supposed to build but they don't really know? Lots of times!)

  My main goal in writing this code was in the mindset that I am in an interview and that I am given only 20 minutes to solve this problem.

It's usually ok to make something work the easiest possible way and then gradually make it better. In this example the easiest solution seems to be to iterate into a set and then convert that to an array.

public int[] removeDuplicate(int[] arr) {
    return toIntArray(toSet(arr));
}

private Set<Integer> toSet(int[] arr) {
    Set<Integer> set = new LinkedHashSet<>();
    for (int num : arr) {
        set.add(num);
    }
    return set;
}

private int[] toIntArray(Set<Integer> set) {
    int[] result = new int[set.size()];
    int i = 0;
    for (int num : set) {
        result[i++] = num;
    }
    return result;
}

Code review

Using the boolean literals true and false in conditions like this can be a red flag:

if (check[i] != true)

Use boolean expressions directly:

if (!check[i])

This is manual array copy:

    int returnArray[] = new int[index];

    for(int i = 0; i < returnArray.length; i++)
        returnArray[i] = temp[i];

    return returnArray;

It's recommended to use System.arraycopy or Arrays.copyOf instead.
In this example you can replace the above lines with this one:

    return Arrays.copyOf(temp, index);

temp is a very poor name. Almost anything would be better, how about unique. With index renamed to uniqueCount, you end up with something quite natural:

    return Arrays.copyOf(unique, uniqueCount);


Basic calculator program in Python as a beginner I'm a beginning coding and this is my very first Python program - a basic calculator. I would very much appreciate any recommendations or fixes!

if restart != "Y":
        print("It was a pleasure calculating with you!")
        time.sleep(1)
        print("\nGoodbye!")
        time.sleep(1)
        sys.exit()

# Modules:

import time     # Provides time-related functions
import sys      # Used to exit the program when needed

# Operations:

def add(num1, num2):
    #   Returns the sum of num1 and num2
    return num1 + num2

def sub(num1, num2):
    #   Returns the difference of num1 and num2
    return num1 - num2

def mul(num1, num2):
    #   Returns the product of num1 and num2
    return num1 * num2

def div(num1, num2):
    #   Returns the quotient of num1 and num2
    try:
        return num1 / num2
    except ZeroDivisionError:
        # Handles division by zero
        print("Division by zero cannot be done. You have broken the universe. Returning zero...")
        return 0

def exp(num1, num2):
    #   Returns the result of num1 being the base and num2 being the exponent
    return num1 ** num2

# Run operational functions:

def run_operation(operation, num1, num2):
    # Determine operationz
    if operation == int("1"):
        print("Adding...\n")
        time.sleep(1)
        print(num1, "+", num2, "=", add(num1, num2))
    elif operation == int("2"):
        print("Subtracting...\n")
        time.sleep(1)
        print(num1, "-", num2, "=", sub(num1, num2))
    elif operation == int("3"):
        print("Multiplying...\n")
        time.sleep(1)
        print(num1, "*", num2, "=", mul(num1, num2))
    elif operation == int("4"):
        print("Dividing...\n")
        time.sleep(1)
        print(num1, "/", num2, "=", div(num1, num2))
    elif operation == int("5"):
        print("Exponentiating...\n")
        time.sleep(1)
        print(num1, "^", num2, "=", exp(num1, num2))
    else:
        print("I don't understand. Please try again.")
        time.sleep(1)

def main():
    while True:                   # Loops if user would like to restart program
        valid_input = False
        while not valid_input:    # Loops if user enters an invalid input. Avoids error
            try:
                # Acquire user input
                num1 = (int(input("What is number 1? ")))
                num2 = (int(input("What is number 2? ")))
                operation = int(input("What would you like to do? \n1. Addition, 2. Subtraction, 3. Multiplication, "4. Division, 5. Exponentiation \nPlease choose an operation: "))
                valid_input = True
            except ValueError:      # Handles any value errors
                print("Invalid input. Please try again.")
                time.sleep(1)
        run_operation(operation, num1, num2)
        time.sleep(1)
        # Ask if the user wants to do more calculations or exit:
        restart = (str(input("Would you like to make more calculations? Enter (Y) for yes, or any other character for no. ")))

I'm having a lot of trouble with the code block below. It doesn't recognize "Y" and instead outputs: "It was a pleasure calculating with you! Goodbye!" What could I do?

if restart != "Y":
    print("It was a pleasure calculating with you!")
    time.sleep(1)
    print("\nGoodbye!")
    time.sleep(1)
    sys.exit()

#  Title Sequence
print('\n\nCalculator\n\n')
time.sleep(1)
print('      ---LOADING---\n\n')
time.sleep(2)
print('Hello. I am your personal calculator. \nBEEP BOOP BEEP. \n\n')
time.sleep(1)

#  Calls the main function to start program
main()

# Turns my program into an importable module
if __name__ == '__calculator__':
    main()

Thanks for asking your question! I'll try and give a concise answer to the main issues with your code. Overall it is well done for a beginner.

First, I'll address the code block you're having trouble with. The restart variable is never defined, so I'm not even sure how your code ran at all. You will need to give restart some initial value, supposedly "Y" as that would have it skip over that code in the beginning.

There is a bigger problem here though, which is that you are putting code in the global scope and also in a main function. Usually, Python scripts do one or the other, as they mean different things to other Python programmers. Although it is completely valid what you're doing, it's not good Python. The purpose of the main function and the if __name__ == '__main__' conditional is so that you can separate the "executable" part of your program from the "library" part.

(See this StackOverflow answer for more detail: https://stackoverflow.com/questions/419163/what-does-if-name-main-do)

The main function allows your code to have different behavior when your program is run on its own, as opposed to when it's imported and usually this means that the imported code does not run anything on its own especially the blocks you're having trouble with.

I would suggest putting all the "running" code, that is, the restart check, and the "calculator startup" all in the main function. And, if you want to logically separate these ideas, then put them all in separate functions that run from main. Something perhaps along the following lines:

def restart(response):
    # uses "in" to check multiple values,
    # a replacement for (response == "Y" or response == "y")
    # which is longer and harder to read.
    if response in ("Y", "y"):
        return True
    else:
        # statements you want to print to the screen before exiting
        ...
        return False

And in the main function:

def main():
    ...
    setup_calculator()  # something you would write, feel free to give it a better name
    ...
    restart_msg = "Would you like to make more calculations?" \
                  "Enter (Y) for yes, or any other character for no. "
    if not restart(str(input(restart_msg))):  # uses the function I wrote
        return  # would not need to import "sys", just exit from main normally

You can do a similar thing for the calculator startup, but I'll leave that to you.

In your run_operation function, you write conditionals with int("1"), int("2"), ... etc. You can just write 1, 2, ... etc, for each if statement. Also, you don't have to worry about typecasting the inputs to run_operation because you do that in the input in main. Once you have the types from the user input, assume that your code runs on that specific type.

One more thing I noticed is that almost everytime you print to the console, you also perform a time.sleep(1), (in one case a time.sleep(2)). What I suggest, so that you don't repeat yourself, is to write your own print function that also performs a sleep every time,

def calculator_print(*args, delay=1):
    print(*args)
    time.sleep(delay)

You could give it a better name, I'm just concerned with the form that it has. This would probably halve your line count and improve readability by many times. The *args is a way for you to accept any number of arguments to calculator_print to match the way that print itself can and does accept many arguments, like in your run_operation function. The delay default argument is there to set the sleep delay to a different value other than 1 like in your calculator startup. This will get rid of all of your explicit time.sleep calls and abstracts away the details of the printing itself. (Say you wanted to change it to do something different, you can still use the rest of the code you've written in the same way. You would only have to change the calculator_print function. Encapsulation is good!)

If you fix these few problems, I think you will be rid of any bugs that you have already and your readability will skyrocket. A very very important part of programming that is grossly overlooked is readability. This should always be taught alongside syntax. As a programmer you will be reading code more than writing it so you should be good at reading good (and bad) code and writing code that makes it easier for others to read; you may even be reading your own old code one day. Writing comments and clear code are both essential for readability.

You've done a really good job here as a beginner. Let me know if you have any questions about what I've said or about anything else Python.

Keep up the good work!


Classes and variables in PHP First of all, I'm trying to learn OOP PHP, so any advise will be great.

My idea is to create some kind of MVC Framework CMS kind of thing, just to learn OOP PHP and MVC well.

Let's say that I've got this code (note: I will separate the code pieces in different files, but this was just for testing):

<?php

function printExtender($var) {
    $returnVal = '<pre>';
    $returnVal .= print_r($var, TRUE);
    $returnVal .= '</pre>';
    return $returnVal;
}

class Main_Controller {

    protected $_vars = array();
    protected static $_instance;

    private function __construct() {
    }

    public static function getInstance() {
        if (!isset(self::$_instance)) {
            self::$_instance = new self();
        }
        return self::$_instance;
    }

    public function & __get($name) {
        return $this -> _vars[$name];
    }

    public function __set($name, $value) {
        $this -> _vars[$name] = $value;
    }

    public function loadDefaultClasses(){
        $main = Main_Controller::getInstance();
        $main -> config = Config::getInstance();
        $main -> sql = SQL::getInstance();
    }
}

class Config {
    protected $_vars = array();
    protected static $_instance;

    private function __construct() {
        $this-> db = array("localhost");
    }

    public static function getInstance() {
        if (!isset(self::$_instance)) {
            self::$_instance = new self();
        }
        return self::$_instance;
    }

    public function & __get($name) {
        return $this -> _vars[$name];
    }

    public function __set($name, $value) {
        $this -> _vars[$name] = $value;
    }

}

class SQL{
    protected static $_instance;

    private function __construct() {
        $main = Main_Controller::getInstance();
        echo printExtender($main -> config -> db);
    }

    public static function getInstance() {
        if (!isset(self::$_instance)) {
            self::$_instance = new self();
        }
        return self::$_instance;
    }
}

$main = Main_Controller::getInstance();
$main -> loadDefaultClasses();

The idea was to get the $main -> config -> .... thing working so I can use that later on. Am I doing this right, or is it awful? And do you have any tips?

Overview
This isn't OOP. To me, object-oriented programming requires thinking about your programs in terms of classes, their responsibilities, their attributes, and behaviours. This means:

Information hiding
Encapsulation
Polymorphism
Collaboration

Config Class
Dearth of source code comments aside, let's look at the Config class. Here are some questions about the configuration options:

What are the options?
What do each of the options do?
How do the options affect the application?
How are the options communicated to the end user?

In this case, the Config "class" as written is nothing more than a functional structure (i.e., it is a dynamic array) wrapped in the syntax of a PHP class. A class, by definition, allows instances to have state and behaviour. The Config class has no discernible behaviour. To give it behaviour, we'd have to contemplate the purpose of the class: its raison d'être.
The name "Config" indicates that the class is responsible for some kind of configuration. (Note: I absolutely abhor abbreviations.) I would further imagine that the configuration is specifically for the CMS application. So I would have named the class CMSConfiguration, which inherits from a more generic Configuration class. Let's look at that generic class.
What can an application configuration class do? I imagine a general configuration class can do the following:

Load options (from a stream, a file, command line switches)
Save options (to maintain current state, or save preferences)
Answer whether an option is set
Answer whether an option is equal to a given value
Export help for the options in an display-device-neutral markup language

That's its behaviour. The single responsibility is clear: the Configuration class loads and queries the state of configuration options. The implementation might resemble:
<?php
/**
 * Responsible for loading and saving configuraiton options.
 */
class Configuration {
  /**
   * Loads the configuration options from application's database connection.
   */
  public function load() {
    Database database = Database::getInstance();
    $result = database->call( "get_configuration_options" );

    // Set the configuration options using the result set.
  }

  /** Saves the current state of the configuration options. */
  public function save() {
  }

  /**
   * Returns true to indicate that the option with the given name is set.
   *
   * @param $option The name of the option to check.
   * @return true The option has a value that is not false.
   */
  public function isOptionSet( $option ) {
    return $this->getOption( $option );
  }

  /** ... */
  public function isOptionEqual( $option, $value ) {
    return $this->getOption( $option ) === $value;
  }

  /** Writes the help for all the options as an XML document. */
  public function exportHelp() {
    foreach( $this->getOptions() as $option ) {
      // ...
    }
  }
}
?>

There seems to be a trend of not declaring attributes, but using magical __set and __get functions combined with dynamic arrays. This will lead to unmaintainable code. If the variables that are used by a class are not defined in a single location, how do you expect anyone else to understand how the code works?
You could, for example, have ClassA and ClassB. What is stopping ClassB from using ClassA as its container for attribute values? Nothing. And that's a terrible, terrible thing, as it completely violates both information hiding and encapsulation.
SQL Class
Judging by its name, the SQL Class seems to be some kind of holder for SQL statements? This will directly violate encapsulation again, as the class that needs to use the SQL will have to call the SQL class to get it. The SQL class should know nothing about the SQL that other classes need to perform.
I actually go a bit further than this. I don't like exposing SQL in my applications at all. SQL changes a lot over the lifetime of a project. When you have many different fingers all creating their own SQL statements, you wind up with a mess: duplicate code, inefficient queries, a burdened query cache, brittle code that will break anytime a table changes, and no clean way to unit test the queries.
The problem, in my mind, is that there is no contract prototype between a CRUD (Create, Read, Update, or Delete) statement and the calling code. This means that the two cannot easily vary independently.
To resolve this, I move the SQL code into stored procedures and stored functions (collectively stored routines). Some people will despise this approach, others will laud it. I prefer it because it means I can create a contract between the stored routine and the underlying code. (It also means that if I wanted to switch from PHP to another language, there's less code to change.)
Once all the SQL code is written in stored routines, it becomes trivial to write a generic database class that can call those routines with arbitrary parameters.
<?php
use PDO;
use PDOException;

/**
 * Used for interacting with the database. Usage:
 * <pre>
 * $db = Database::get();
 * $db->call( ... );
 * </pre>
 */
class Database {
  private static $instance;
  private $dataStore;

  /**
   * Sets the connection that this class uses for database transactions.
   */
  public function __construct() {
    global $dbhost;
    global $dbname;
    global $dbuser;
    global $dbpass;

    try {
      $this->setDataStore(
        new PDO( "pgsql:dbname=$dbname;host=$dbhost", $dbuser, $dbpass ) );
    }
    catch( PDOException $ex ) {
      $this->log( $ex->getMessage() );
    }
  }

  /**
   * Returns the singleton database instance.
   */
  public function get() {
    if( self::$instance === null ) {
      self::$instance = new Database();
    }

    return self::$instance;
  }

  /**
   * Call a database function and return the results. If there are
   * multiple columns to return, then the value for $params must contain
   * a comma; otherwise, without a comma, the value for $params is used
   * as the return column name. For example:
   *
   *- SELECT $params FROM $proc( ?, ? ); -- with comma
   *- SELECT $proc( ?, ? ) AS $params; -- without comma
   *- SELECT $proc( ?, ? ); -- empty
   *
   * @param $proc Name of the function or stored procedure to call.
   * @param $params Name of parameters to use as return columns.
   */
  public function call( $proc, $params = "" ) {
    $args = array();
    $count = 0;
    $placeholders = "";

    // Key is zero-based (e.g., $proc = 0, $params = 1).
    foreach( func_get_args() as $key => $parameter ) {
      // Skip the $proc and $params arguments to this method.
      if( $key < 2 ) continue;

      $count++;
      $placeholders = empty( $placeholders ) ? "?" : "$placeholders,?";
      array_push( $args, $parameter );
    }

    $sql = "";

    if( empty( $params ) ) {
      // If there are no parameters, then just make a call.
      $sql = "SELECT $proc( $placeholders )";
    }
    else if( strpos( $params, "," ) !== false ) {
      // If there is a comma, select the column names.
      $sql = "SELECT $params FROM $proc( $placeholders )";
    }
    else {
      // Otherwise, select the result into the given column name.
      $sql = "SELECT $proc( $placeholders ) AS $params";
    }

    $db = $this->getDataStore();
    $statement = $db->prepare( $sql );

    for( $i = 1; $i <= $count; $i++ ) {
      $statement->bindParam( $i, $args[$i - 1] );
    }

    try {
      $result = null;

      if( $statement->execute() === true ) {
        $result = $statement->fetchAll( PDO::FETCH_ASSOC );
        $this->decodeArray( $result );
      }
      else {
        // \todo Send an e-mail.
        $info = $statement->errorInfo();
        $this->log( "SQL failed: $sql" );
        $this->log( "Error: ". $info[2] );
      }
    }
    catch( PDOException $ex ) {
      // \todo Send an e-mail.
      $this->log( $ex->getMessage() );
    }

    return $result;
  }

  /**
   * Converts an array of numbers into an array suitable for usage with
   * PostgreSQL.
   *
   * @param $array An array of integers.
   */
  public function arrayToString( $array ) {
    return "{" . implode( ",", $array ) . "}";
  }

  /**
   * Recursive method to decode a UTF8-encoded array.
   *
   * @param $array - The array to decode.
   * @param $key - Name of the function to call.
   */
  private function decodeArray( &$array ) {
    if( is_array( $array ) ) {
      array_map( array( $this, "decodeArray" ), $array );
    }
    else {
      $array = utf8_decode( $array );
    }
  }

  private function getDataStore() {
    return $this->dataStore;
  }

  private function setDataStore( $dataStore ) {
    $dataStore->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
    $this->dataStore = $dataStore;
  }
}

The code is used in one of three ways:
// Returns a boolean value
$result = $db->call( "is_existing_cookie", "existing", $cookie_value );

// Calls a stored procedure
$this->call( "procedure_insert", "", $this->getId(), $action_name_id );

// Returns a tuple pair of ids and labels
return $this->call( "get_list", "id, label", $this->getId() );

The Database class has behaviours (make a database call) and attributes (the singleton instance). There is certainly room for improvement, but this should give you more ideas to ponder.
All of the text is great, but the example code... That database class violates just about every OOP principle there is. Importing global variables? Constructing a PDO instance in the constructor? A singleton? What? Why? Globals have no place in modern PHP, nor do singletons. Dependency injection is the way to go here. And suppressing exceptions? Nooooo. The consuming code needs to know something exceptional happened. Logging is not enough (and I would argue that either an extension, wrapper or consumer of the class should be responsible for logging).
Response to Corbin

That database class violates just about every OOP principle there is.

The Database class violates neither encapsulation nor information hiding. This is apparent from its intended usage:
return $this->call( "get_list", "id, label", $this->getId() );

The underlying implementation (of the call method) can change without any impact to the client code. No information about the internal workings of the Database class is being exposed.

Importing global variables?

True, this is poor form. However, it can be mitigated by using a configuration file, or dependency injection. Again, the calling classes will not need to change.

Constructing a PDO instance in the constructor? A singleton? What? Why?
Globals have no place in modern PHP, nor do singletons.

True, globals are poor form. The Singleton pattern was employed for its simplicity. In my application, every single class uses the Database, but most of the subclasses do not use it directly.
This code is also fairly temporary, as the application will be rewritten using Haxe.

And suppressing exceptions? Nooooo.

Depends on if the calling code needs them. In my case, the calling code never needs to care about the error. Users don't care if there was a database error. There is no point bubbling an error message to the user, as they'll never understand "ORA-10168: INSERT violates duplicate key" or whatever. Ergo, the message is suppressed and the application must handle an empty result set.
In this particular case, all empty result sets yield a relatively empty XML document, which is rendered using XSLT, to produce a web page for the user. There is little difference between the following:
try {
  $result = $this->call( ... );
}
catch( Exception $e ) {
  $result = $this->getErrorXML();
}

And this:
$result = $this->call( ... );

if( !isset( $result ) ) {
  $result = $this->getErrorXML();
}

They are functionally equivalent, although the exception handling allows slightly more flexibility for eliminating duplicate code (by allowing the exception to bubble naturally and get handled in a single location).
As I mentioned, there are a number of minor issues with the Database class, however it does not violate either encapsulation or information hiding.
From a big picture perspective, I'd rather see:
return $this->call( "get_list", "id, label", $this->getId() );

Sprinkled throughout the codebase, than hundreds of SELECT statements married to PDO calls.


Calculate the square root of the sum of the squares of the prime factors of an input number This question is related to another one that I have asked here.

I need to find ways to improve the runtime performance of the following piece of C-code.

I want to increase RANGE to around 1G, and still complete the execution within a reasonable time.

Auxiliary data structures:

#define RANGE 16000000

uint08 sieve[RANGE]    = {0};
uint32 prime[RANGE/16] = {0}; // only around RANGE/log(RANGE) are primes
uint32 numOfPrimes     =  0 ;

Initialize the auxiliary data structures:

void CalcAuxiliaryData()
{
    uint32 i,j;

    uint32 root = (uint32)sqrt((double)RANGE);

    for (i=2; i<=root; i++)
    {
        if (sieve[i] == 0)
            for (j=i+i; j<RANGE; j+=i)
                sieve[j] = 1;
    }

    for (i=2; i<RANGE; i++)
    {
        if (sieve[i] == 0)
            prime[numOfPrimes++] = i;
    }
}

Calculate the square root of the sum of the squares of the prime factors of an input number:

uint32 CalcDiagonalLen(uint32 n)
{
    uint32 i;

    uint64 square;
    uint32 length;

    if (sieve[n] == 0) // quickly resolve the case of a prime number
        return n;

    square = 0;
    for (i=0; i<numOfPrimes && n>1; i++)
    {
        uint32 p = prime[i];
        uint64 pp = (uint64)p*p;
        while (n%p == 0)
        {
            n /= p;
            square += pp;
        }
    }

    length = (uint32)sqrt((double)square);
    if ((uint64)length*length == square)
        return length;

    return 0; // indicate that the result is not integer
}

Calculate the square root of the sum of the squares of the prime factors of each number:

int main()
{
    uint32 i;

    uint32 diagonal_len;

    CalcAuxiliaryData();

    for (i=2; i<RANGE; i++)
    {
        diagonal_len = CalcDiagonalLen(i);
        if (diagonal_len != 0)
            printf("%u %u\n",i,diagonal_len);
    }

    return 0;
}

Function CalcDiagonalLen holds the bottleneck, but any suggestions will be greatly appreciated.

You can use the sieve approach to create an array of square sums. For any new prime that you sieve, you have:

sqsum[m * p] = sqsum[m] + p*p;

You can build that array as you go. The primes need a square sum value, too.

uint8_t sieve[RANGE] = {0};
uint64_t sqsum[RANGE] = {0};

void CalcAuxiliaryData()
{
    uint32_t i, j, k;

    for (i = 2; i <= RANGE; i++) {
        if (sieve[i] == 0) {
            uint64_t ii = (uint64_t) i * i;

            sqsum[i] = ii;

            j = 2;
            k = i + i;
            while (k < RANGE) {
                sieve[k] = 1;
                sqsum[k] = sqsum[j++] + ii;
                k += i;
            }
        }
    }
}

uint32_t CalcDiagonalLen(uint32_t n)
{
    uint64_t square;
    uint32_t length;

    if (sieve[n] == 0) return n;

    square = sqsum[n];
    length = sqrt(square);
    if ((uint64_t) length * length == square) return length;

    return 0;
}

This approach adds more time to the initialisation, because you can't cut sieving short at sqrt(RANGE), but the sum lookup is fast.

You could also create a lookup array that tells you whether a number is a perfect square or not, but the sqrt function isn't the bottleneck here. And you could make the array itself a direct lookup table for the diagonal length.


Kadane's Algorithm for 2D array with known boundaries I asked this question first on StackOverflow but I didn't get an answer and was advised to try here too. So here we go.

I have implemented Kadane's algorithm for a 2D array in Python 2 with known boundaries, but I'm using the implementation for an online contest and the time it takes is more than the time given.

So that made me think if there is maybe another algorithm similar to Kadane's that has a smaller complexity, or if my code can be optimized in a way. My implementation works for any array with dimensions N x M and a subarray with dimensions maxRows x maxCols.

maxSumSubarray.py

import numpy as np

# returns the maximum sum for the given vector using kadane's algorithm, with
# maxRows maximum members in the sum
def kadane1DwithBounds(maxRows):
    global temp
    m = s = sum(temp[i] for i in xrange(maxRows))
    k = 0

    for i in xrange(1, N - maxRows + 1):
        s -= temp[k]
        s += temp[maxRows + i - 1]
        k += 1
        m = max(m, s)

    return m

# prints the maximum "area" given by the values of an NxM array inside a
# subarray with dimensions maxRows x maxCols. temp holds the latest vector to be
# given to kadane1DwithBounds()
def kadane2DwithBounds(maxRows, maxCols):
    global temp
    for i in xrange(N):
        temp[i] = sum(table[i][j] for j in xrange(maxCols))

    m = kadane1DwithBounds(maxRows)

    k = 0
    for j in xrange(1, M - maxCols + 1):
        for i in xrange(N):
            temp[i] -= table[i][k]
            temp[i] += table[i][maxCols + j - 1]
        k += 1
        m = max(m, kadane1DwithBounds(maxRows))

    print m

line = map(int, raw_input().split())
N = line[0]
M = line[1]
maxRows = line[2]
maxCols = line[3]

table = []
temp = np.empty(N, dtype = int)

for _ in xrange(N):
    table.append(map(int, raw_input().split()))

kadane2DwithBounds(maxRows, maxCols)

test.txt

4 8 2 3
1 1 2 3 3 1 1 1
2 2 2 2 2 2 2 2
3 3 3 1 1 3 3 4
0 0 1 1 3 2 2 1

Run with

python maxSumSubarray.py < test.txt

it gives

16

which is correct and is the following rectangle:

2 2 2
3 3 4

I've tried with other dimensions too and I'm pretty sure it works fine. Only problem is time/complexity.

Input can be simplified using destructuring assignment and NumPy:

N, M, max_rows, max_cols = map(int, raw_input().split())
table = np.genfromtxt(sys.stdin)

By the PEP 8 standard, variable and function names should be written using lowercase_with_underscores.  The comment block for each function should be a docstring instead.

The kadane2DwithBounds() function should return its result instead of printing it.

The use of global variables N, M, table, and temp within the two functions is disconcerting.  table and temp should be passed explicitly.  N and M can be inferred by inspecting the table itself, and therefore don't need to be passed to the kadane functions.


Geocoding addresses from a SAP database I'm new to python and curious if this is the most efficient way to read addresses from a SAP database, geocode them, read the results, and then write the information back to the database.
Everything below works, I'm just curious if this would make an expert cry beginner bad, or acceptable code for my first try. :-) I'm pretty sure my variable name formatting isn't 100% up to spec.
For reference on Geocodio, https://www.geocod.io/docs/?python#fields
import platform
import json
import sys
from hdbcli import dbapi
from geocodio import GeocodioClient

client = GeocodioClient('x')

conn = dbapi.connect(address='x', port=x, user='x', password='x')

cursor = conn.cursor()
sql_command = "SELECT ADDRNUMBER, CITY1, REGION, POST_CODE1,STREET FROM CV_COORDS_TO_PROCESS"
cursor.execute(sql_command)
rows = cursor.fetchall()

address = ''
for row in rows:
    #build the address string and save the address id
    address = row[4] +", " + row[1] +", " + row[2] +" " + row[3]
    addr_id = row[0]
    #send the address to geocodio
    location = client.geocode(address)

    #Read the results and break after the first result, as per the geocodio documentation,
    #in the event of multiple points, it lists them in order of confidence
    for subs in location['results']:
        lat = subs['location']['lat']
        long = subs['location']['lng']
        accuracy = subs['accuracy']
        accuracy_type = subs['accuracy_type']
        break

    ##update / insert into database
    merge_query = "upsert zsa_addr_coords values  ('100', %s, %f, %f, %f, '%s')"\
                  %(addr_id, lat, long, accuracy, accuracy_type ) + " where addr_id = '%s'" %addr_id
    try:
        cursor.execute(merge_query);
    except:
        print (ex)

    address = ''
cursor.close()
conn.close()

This is example of the results Geocodio returns:
{
  "input": {
    "address_components": {
      "number": "1109",
      "predirectional": "N",
      "street": "Highland",
      "suffix": "St",
      "formatted_street": "N Highland St",
      "city": "Arlington",
      "state": "VA",
      "zip": "22201",
      "country": "US"
    },
    "formatted_address": "1109 N Highland St, Arlington, VA 22201"
  },
  "results": [
    {
      "address_components": {
        "number": "1109",
        "predirectional": "N",
        "street": "Highland",
        "suffix": "St",
        "formatted_street": "N Highland St",
        "city": "Arlington",
        "county": "Arlington County",
        "state": "VA",
        "zip": "22201",
        "country": "US"
      },
      "formatted_address": "1109 N Highland St, Arlington, VA 22201",
      "location": {
        "lat": 38.886665,
        "lng": -77.094733
      },
      "accuracy": 1,
      "accuracy_type": "rooftop",
      "source": "Virginia GIS Clearinghouse"
    }
  ]
}

In Python, you don't need to create your variables before using them, so the address = '' before your loop is unnecessary. You also don't need to reset a variable before using it again, so the address = '' at the end of each loop iteration is particularly unnecessary.
You currently get the addresses from the DB and then concatenate parts of that address as the query string for your geolocation. This would be easier if you got the address in the same order you need it later:
sql_command = "SELECT ADDRNUMBER, STREET, CITY1, REGION, POST_CODE1 FROM CV_COORDS_TO_PROCESS"
cursor.execute(sql_command)
rows = cursor.fetchall()

for row in rows:
    #build the address string and save the address id
    addr_id, address = row[0], ", ".join(row[1:])
    ...

There's probably a way to do the concatenation already in SQL, but I am not versed enough in that language to suggest how.
If you want to get the first value from an iterable, you can use next:
location = next(iter(client.geocode(address)['results']))

Most DB wrappers support prepared statements, which would mean that you don't have to prepare the full query string, but rather use something like this:
merge_query = "upsert zsa_addr_coords values ('100', ?, ?, ?, ?, ?) where addr_id = ?"
try:
    cursor.execute(merge_query, (addr_id, lat, long, accuracy, accuracy_type, addr_id));
except:
    print (ex)

Indeed, hdbcli also support this: https://blogs.sap.com/2017/07/26/sap-hana-2.0-sps02-new-feature-updated-python-driver/
It should even take care of quoting for you and prevents the most basic SQL
injection attacks.
And since that string now never changes, you can pull it outside of the loop.
You can probably speed up the execution time by disabling the auto-commit mode before your loop and manually committing at the end, which ensures that you don't have to do that every loop iteration:
conn.setautocommit(False)
for row in rows:
    ...
conn.commit()


Find primes that are also primes when its digits are rotated The program below is meant to find all of the fare prime numbers under a user specified value. A fare prime being a number that when its digits are rotated, each combination makes a prime number. An example being 113 because 113, 311, and 131 are all prime numbers. My current problem is that the program takes a very long time to process very large numbers so I need a way of making it run quicker. I've tried to explain the code with the comments but if any part doesn't make sense I'll do my best to explain.

#get user input for the number range
n = int(input("number"))
primes = []
#find all the prime numbers up to the specified value and append them to a list
for num in range(2, n+1):
  for i in range(2, num):
      if num % i == 0:
          break
  else:
     primes.append(num)

#find out if the prime number is a fare prime
for i in primes:
  length = len(str(i))
  #if the number has one digit it is automatically a fare prime
  if length == 1:
    print(i)
  #if the number is longer, rotate the digits to see if it is a fare prime
  if length >= 2:
    number = i
    fare_primes = []
    #rotate the number to figure out if all combinations are prime
    for j in range(length):
      #turns # into a list of digits
      digit_list = list(str(number))
      #rearranges
      number = [*digit_list[1::], digit_list[0]]
      part = ""
      #turns it back into an int value
      number = part.join(number)
      int_num = int(number)
      #check if # is prime
      for divider in range(2,int_num):
        if int_num % divider == 0:
          break
      else:
        fare_primes.append(number)
    #if all combinations of the digits are prime the original # is printed
    if len(fare_primes) == length:
      print(fare_primes[-1])

A 2+ digit fare prime number can never have an even digit, or a 5 digit in its set of digits, because a rotation which moves an even digit or a 5 to the last digit will be divisible by 2 or 5.  You could use that as a filter for your possible fare primes from the list of primes you calculate.

When calculating primes, you can stop at sqrt(num).  Any number greater than sqrt(num) that evenly divides num will have a compliment number less than sqrt(num) that you have already tested.

Speaking of primes that you’ve calculated, why don’t you use those for your subsequent prime test?  Why try dividing by every number from 2 up to int_num when you could just try the numbers in primes upto int_num.

... or just ask if int_num in primes.  Speed tip: turn primes into a set() first for faster inclusion testing.

Your digit_list code is very inefficient. For any number, once you’ve split the number into digits, you don’t need to resplit it into digits again for each rotation.  Actually, you don’t even need to split it into individual digits.  This will give you the rotated values:

digits = str(i)
for j in range(1, length):
    int_num = int( digits[j:] + digits[:j])


How and where to define a partial helper module I want to split my helpers in different modules (and files), but I found some problems which I got around with the following solution:

# app/helpers/application_helper.rb
module ApplicationHelper
  include ApplicationContainer
  # ...
end

# app/helpers/partials/application_container.rb
module ApplicationContainer
  # ...
end

In particular at the beginning I was looking to have a file /app/helpers/_application_container_helper.rb that gave me uninitialized constant ApplicationHelper::ApplicationContainer (and different other errors playing around it), but right now I'm not even sure that makes sense.
Any Idea or suggestions?

Your current solution is fine, in my opinion. The underscore naming-convention used on view-partials doesn't apply for modules, that's why you got the uninitialized constant-error. If you name it ApplicationHelper::ApplicationContainer it has to be in a file named application_container.rb

What you have in your code now is a module you can include in other Helper-modules– just as you would a partial in your views.