Single MySQLI query to check for admin privileges I have this script on top of all pages to check if user is admin otherwise we kick him. The problem is that I don't know if I am using double resources with a double query and results. Can I make this code to be only 1 query and 1 result? Please read the comments in code to understand:

<?php
session_start();
$identification = $_SESSION['session_name'];
include 'inc/database.php';
$query = "SELECT * FROM `members` WHERE `username` = '$identification' LIMIT 1";

// Here is the question about, we run first query to verify user

$result_1 = mysqli_query($database, $query);

// Now I am like obligated to query 2 times and get 2 results to make the While user_info $result_2 to work

$result_2 = mysqli_query($database, $query);

$privileges = mysqli_fetch_array($result_1, MYSQLI_ASSOC);
$user_level = $privileges["privs"];
if ($user_level=='ADMIN'){}
else {
header('Location: security.php?error=missing correct privileges. user is not admin');
exit();
}

// If I set here to $result_1 it just does not work, I feel obligated to query 2 times and use more resources

while($user_info = mysqli_fetch_assoc($result_2))
{
?>
<html>
<head>
<title>Zona de Pruebas</title>
</head>
<body>
Username: <?php echo $user_info['username']?>, can be here because is admin</div>
</body>
</html>
<?php
}
?>

You do not need to re-fetch or use a while loop, simply reuse the array that you fetched the first time.

<?php
session_start();
$identification = $_SESSION['session_name'];
include 'inc/database.php';
$query = "SELECT * FROM `members` WHERE `username` = '$identification' LIMIT 1";

$result = mysqli_query($database, $query);

$user_info = mysqli_fetch_array($result, MYSQLI_ASSOC);
$user_level = $user_info["privs"];
if ($user_level=='ADMIN'){}
else {
  header('Location: security.php?error=missing correct privileges. user is not admin');
  exit();
}

?>
<html>
<head>
<title>Zona de Pruebas</title>
</head>
<body>
Username: <?php echo $user_info['username']?>, can be here because is admin</div>
</body>
</html>
<?php
}
?>


Optimizing a function in Eigen I'm a beginner in C++ and I would appreciate advices to optimize the following function I wrote with Eigen (in fact, to be used with RcppEigen).
So far, I observe a 3.5x speed-up compared to the corresponding function written in R, and I was wondering whether I could gain more.

Note that I'm working with very large matrices, so I rely on maps to avoid copies from the corresponding objects in R.

Thanks in advance for your help!

#include <RcppEigen.h>

using namespace Rcpp;
using namespace Eigen;

typedef Map<ArrayXd> MapArr1D;
typedef Map<ArrayXXd> MapArr2D;
typedef Map<MatrixXd> MapMat;
typedef Map<VectorXd> MapVec;

// [[Rcpp::depends(RcppEigen)]]

// [[Rcpp::export]]
void myFct(const MapMat M1, const MapMat M2, MapMat M3, MapMat M4, MapArr2D A1,
           MapArr2D A2, const MapArr1D a1, const MapArr1D a2, const MapArr1D a3,
           const MapArr1D a4, const MapArr1D a5, const double d1) {

 for (int j = 0; j < M1.cols(); ++j) {

    M4.noalias() -= M1.col(j) * M3.row(j);

    A1.row(j) = a1 * a2 * ((M2 - M4).transpose() * M1.col(j)).array();

    A2.row(j) = exp(-Fct(a3(j) - a4(j) - a5 / 2 - d1 / 2 -
      pow(A1.row(j).transpose(), 2) / (2 * a1) - log(a1) / 2));

    M3.row(j) = A1.row(j) * A2.row(j);

    M4.noalias() += M1.col(j) * M3.row(j);

  }
}

where Fct is some other function.

It's hard to help without knowing the sizes, but:

The first thing to do would be to measure the relative cost of each of the 5 statements to see where the bottleneck is.
-(a5/2 + d1/2 + log(a1)) could be precomputed into a temporary outside the loop.
Replace pow(A1.row(j).transpose(), 2) by A1.row(j).transpose().square().
Compilation flags might also help: make sure you enabled AVX and if supported FMA using e.g., -march=native.
Depending on the sizes, you might want to rewrite the expressions to benefits from faster matrix-matrix products:

'

T = (M2-M4).transpose() * M1;
for(j...)
  ...
  A1.row(j) = a1*a2 * (T.col(j) - M1.col(j).squaredNorm() * M3.row(j).transpose());
  ...


Phrases update query My query is this:

UPDATE `phrases`
SET `phrases`.`count`=(SELECT COUNT(*) FROM `strings` WHERE `string` LIKE CONCAT('%', `phrases`.`phrase`, '%'))

My tables look like this:

CREATE TABLE `phrases` (
    `hash` varchar(32) NOT NULL,
    `count` int DEFAULT 0,
    `phrase` text NOT NULL,
     PRIMARY KEY (`hash`),
     KEY(`count`)
)

And

CREATE TABLE `strings` (
    `string` text NOT NULL,
)

phrases has 18,000 rows and strings has 1500 rows.

Since you're using a LIKE with wildcards, you're going to do a table-scan against both tables, running a total of 18000*1500 = 27000000 substring comparisons.

To optimize this, you need to use some fulltext index technology.  I suggest Sphinx Search or Apache Solr.  If you do this, you don't need to keep a count of how many matches there are, because the search index makes it a lot less expensive to get a count on demand.

MySQL also implements a FULLTEXT index type, but it is only supported in the MyISAM storage engine in current versions (up to 5.5).  I don't recommend using MyISAM for important data.

MySQL 5.6 is developing a fulltext index for InnoDB.


Beginner's Knight's tour in Python (BFS) I wanted to implement this as short and effective as I could without using anything that is not basics, in order to improve my skills.

I would appreciate input about things like memory leaks I missed, simpler way of doing things instead reinventing the wheel, methods I should have used / implemented / not implemented (could have used built-ins and defaults...) and styling. In general, does this implementation follows common best practices?

import random as r
import itertools as it
import numpy as np

class P:

    up = 8
    down = 0

    def __init__(self,x=0,y=0,parent=None, random = False):
        self.x = x if not random else r.randint(P.down,P.up-1)
        self.y = y if not random else r.randint(P.down,P.up-1)
        self.parent = parent

    def __str__(self):
        return '({},{})'.format(self.x,self.y)

    def __repr__(self):
        return '({},{} [{},{}])'.format(self.x,self.y,self.parent.x if self.parent else '-',self.parent.y if self.parent else '-')

    def __eq__(self,other):
        return self.x == other.x and self.y == other.y

# for np.unique
    def __gt__(self,other):
        return self.x + self.y > other.x + other.y
# for np.unique
    def __lt__(self,other):
        return self.x + self.y < other.x + other.y

    def __hash__(self):
        return hash((self.x, self.y))

    def valid_moves(self):
        def valid(*num):
            # a better way to check if all number in num return True?
            # this seems nice and short to me, but I guess there is a better way
            return sum([n >= P.down and n < P.up for n in num]) // len(num)

        a,b = [1,-1,],[2,-2]
        # is there a way to shorten this list construction?
        t = [P(self.x + i, self.y + j, self) for i in a for j in b if valid(self.x+i,self.y+j)] + [P(self.x + i, self.y + j, self) for i in b for j in a if valid(self.x+i,self.y+j)]
        return np.unique(t)

s = P()
e = P(random = True)
print('findint shortest path from {} to {}'.format(s,e))

ls = s.valid_moves()

while True:
    if e in ls or e == s:
        print('found end node...')
        #find the end node that has parents - not the
        #original "e" that has no parents andt herefore
        #cannot be used to generate path
        curr = ls.pop(ls.index(e))
        path = [curr]
        print('generating path...')
        while curr != s:
            curr = curr.parent
            path += ['->']
            path += [curr]

        print('path: ',path)
        break

    tmp = [p.valid_moves() for p in ls]
    # flatten tmp into 1-d list
    ls = list(it.chain(*tmp))
    # Do I have any memory leaks?
    del tmp

For a beginner this is pretty good, there are some serious styling issues though which I would like to point out.

Review

imports

.Only import what you need, ==> from random import randint
Doing import random as r does not help readability IMO

Constants should be UPPERCASE, rename UP and DOWN accordingly

See PEP8#constants

def valid(*num):

This could be made simpler with the all() keyword

return all(n in range(P.DOWN, P.UP) for n in num)

Here all() checks if all points are in range and will return True if it is, else it will return False

t = [P(self.x + i, self.y + j, self) ...

This list comprehension is way too long, and you actually repeat yourself

A few fixes are needed

It is good to split up your lines for better readability like this:

[P(...)
 for i in a
 for j in b
 ...]

You loop over [-1, 1], [2, -2] with 2 contatinated list comprehensions you could do this in one turn. By having a constant of all possible directions for instance, this will make it look a little better.

I end up with this:

DIRECTIONS = ((1, 2), (1, -2), (-1, 2), (-1, 2), (2, -1), (2, 1), (-2, 1), (-2, -1))
paths = [P(self.rank + delta_rank,
           self.file + delta_file,
           self)
         for delta_rank, delta_file in P.DIRECTIONS
         if valid(self.rank + delta_rank, self.file + delta_file)]

Always guard your code!
There is the issue of naming.
You have many unmeaningfull names

class P() What is P? Point?

a, b This doesn't say much

e, s I would rename to end, start

etc (t, ls, ...)

In chess x is called a RANK and y is called a FILE

When writing good code, it helpfull to give your variables and instances good names, where it is clear on first sight what they do. So when you revisit your code, or let other people use your code they know what is what.

Revised code

from random import randint
from itertools import chain
import numpy as np

class P:
    UP = 8
    DOWN = 0
    DIRECTIONS = ((1, 2), (1, -2), (-1, 2), (-1, 2), (2, -1), (2, 1), (-2, 1), (-2, -1))

    def __init__(self, rank=0, file=0, parent=None, random=False):
        self.rank = rank if not random else randint(P.DOWN, P.UP-1)
        self.file = file if not random else randint(P.DOWN, P.UP-1)
        self.parent = parent

    def __str__(self):
        return '({},{})'.format(self.rank, self.file)

    def __repr__(self):
        return '({},{} [{},{}])'.format(self.rank,
                                        self.file,
                                        self.parent.rank if self.parent else '-',
                                        self.parent.file if self.parent else '-')

    def __eq__(self, other):
        return self.rank == other.rank and self.file == other.file

    def __gt__(self, other):
        return self.rank + self.file > other.rank + other.file

    def __lt__(self, other):
        return self.rank + self.file < other.rank + other.file

    def __hash__(self):
        return hash((self.rank, self.file))

    def valid_moves(self):
        def valid(*num):
            return all(n in range(P.DOWN, P.UP) for n in num)

        paths = [P(self.rank + delta_rank,
                   self.file + delta_file,
                   self)
                 for delta_rank, delta_file in P.DIRECTIONS
                 if valid(self.rank + delta_rank, self.file + delta_file)]

        return np.unique(paths)

def find_path(start, end):
    valid = start.valid_moves()
    while True:
        if end in valid or end == start:
            curr = valid.pop(valid.index(end))
            path = [curr]

            while curr != start:
                curr = curr.parent
                path += ['->']
                path += [curr]

            return path

        tmp = [p.valid_moves() for p in valid]
        valid = list(chain(*tmp))

if __name__ == '__main__':
    start, end = P(), P(random=True)
    print('Finding shortest path from {} to {}'.format(start, end))
    print('Path ', find_path(start, end))


Saving ResourceDictionary theme I have the following command bonded to my button, It's supposed detect the current used theme and change it (Dark or Light).

  var existingResourceDictionary = Application.Current.Resources.MergedDictionaries
                .Where(rd => rd.Source != null)
                .SingleOrDefault(rd => Regex.Match(rd.Source.OriginalString, @"(\/MaterialDesignThemes.Wpf;component\/Themes\/MaterialDesignTheme\.)((Light)|(Dark))").Success);

            if (existingResourceDictionary == null)
                throw new ApplicationException("Unable to find Light/Dark base theme in Application resources.");

            if (existingResourceDictionary.Source.ToString().Contains("Light"))
            {
                Settings.Default.Theme = true;
                Settings.Default.Save();
            }
            else
            {
                Settings.Default.Theme = false;
                Settings.Default.Save();
            }

            var source =
                $"pack://application:,,,/MaterialDesignThemes.Wpf;component/Themes/MaterialDesignTheme.{(Settings.Default.Theme ? "Dark" : "Light")}.xaml";
            var newResourceDictionary = new ResourceDictionary() { Source = new Uri(source) };

            Application.Current.Resources.MergedDictionaries.Remove(existingResourceDictionary);
            Application.Current.Resources.MergedDictionaries.Add(newResourceDictionary);

and in the ViewModel (when I run the application) It should detect the last changes made with the button above and apply the theme. I'm basically getting the value from the Settings that I changed above:

   var existingResourceDictionary = Application.Current.Resources.MergedDictionaries
          .Where(rd => rd.Source != null)
          .SingleOrDefault(rd => Regex.Match(rd.Source.OriginalString, @"(\/MaterialDesignThemes.Wpf;component\/Themes\/MaterialDesignTheme\.)((Light)|(Dark))").Success);

            if (existingResourceDictionary == null)
                throw new ApplicationException("Unable to find Light/Dark base theme in Application resources.");

            var source =
                $"pack://application:,,,/MaterialDesignThemes.Wpf;component/Themes/MaterialDesignTheme.{(Settings.Default.Theme ? "Dark" : "Light")}.xaml";
            var newResourceDictionary = new ResourceDictionary() { Source = new Uri(source) };

            Application.Current.Resources.MergedDictionaries.Remove(existingResourceDictionary);
            Application.Current.Resources.MergedDictionaries.Add(newResourceDictionary);

The code is working fine but I feel like I could make it less repetitive and shorter?

You can substitute

if (existingResourceDictionary.Source.ToString().Contains("Light"))
{
    Settings.Default.Theme = true;
    Settings.Default.Save();
}
else
{
    Settings.Default.Theme = false;
    Settings.Default.Save();
}

by

Settings.Default.Theme = existingResourceDictionary.Source.ToString().Contains("Light");
Settings.Default.Save();

to shorten it a bit.


Order and organize objects in an array I need to organize the following objects in a array applying the following rules:

Group all object by source
In each group order them by order property
Rewrite each order property keeping their order starting from 0

I have drafted an initial solution below, the final result is present in the code comment and the script does its job.

But I would like to have your code review specially in term of performance,  as the real world array could contain hundreds of objects.

Please note a new array is created for this example, as I do not need to change the original array in place.

   var data = [
          {
              id: 'g',
              order: '0',
              source: '3-j'
          },
          {
              id: 'c',
              order: '40',
              source: '1-x'
          },
          {
              id: 'd',
              order: '0',
              source: '2-y'
          },
          {
              id: 'b',
              order: '30',
              source: '1-x'
          },
          {
              id: 'e',
              order: '1',
              source: '2-y'
          },
          {
              id: 'h',
              order: '1',
              source: '3-j'
          },
          {
              id: 'a',
              order: '20',
              source: '1-x'
          },
          {
              id: 'f',
              order: '2',
              source: '2-y'
          }
        ];
        var flags = {};
        data.forEach(function (item) {
            flags[item.source] = true;
        });
        var sources = [];
        Object.keys(flags).forEach(function (flag) {
            sources.push(flag);
        });
        sources.sort();
        var newData = [];
        sources.forEach(function (source) {
            var dataFiltered = data.filter(function (item) {
                return item.source === source;
            });
            dataFiltered.sort(function (a, b) {
                return a.order - b.order;
            });
            dataFiltered.map(function (item, index) {
                item.order = index;
            });
            newData = newData.concat(dataFiltered);
        });
        console.log(JSON.stringify(newData));

    /*
       Final result:
        [
            {
                "id": "a",
                "order": 0,
                "source": "1-x"
            },
            {
                "id": "b",
                "order": 1,
                "source": "1-x"
            },
            {
                "id": "c",
                "order": 2,
                "source": "1-x"
            },
            {
                "id": "d",
                "order": 0,
                "source": "2-y"
            },
            {
                "id": "e",
                "order": 1,
                "source": "2-y"
            },
            {
                "id": "f",
                "order": 2,
                "source": "2-y"
            },
            {
                "id": "g",
                "order": 0,
                "source": "3-j"
            },
            {
                "id": "h",
                "order": 1,
                "source": "3-j"
            }
        ]
    */

I think this could be greatly simplified, as you are currently iterating through the input data many more times than is necessary.

There are a number of possible approaches you could take to optimize this.  One option might be to simplify this down to one call to Array.sort() to apply multi-level sorting, followed by iterating the array to reset order values.

// sort data
data.sort(
    (a, b) => {
        if (a.source < b.source) return -1;
        if (a.source > b.source) return 1;
        if (a.order < b.order) return -1;
        if (a.order > b.order) return 1;
        return 0;
    }
);

// iterate data, applying new order value
var currentGroup = null;
var counter = 0;
for (var i = 0, len = data.length; i < len; i++) {
    if (currentGroup !== data[i].source) {
        counter = 0;
        currentGroup = data[i].source;
    } else {
        counter++;
    }
    data[i].order = counter;
}

The sort here would be \$O(n log(n))\$ on average or \$O(n^2)\$ worst case (based on underlying quicksort implementation for sort()).  And the mapping on new count values would execute in \$O(n)\$.  So, you would have \$O(n^2 + n)\$ worst case complexity.

Memory utilization is also probably close to optimal for this approach, as you are modifying data in place for both the sort and the re-writing of order values.

If you wanted to consider alternate output data structures such as the following, where you use index position in array as "order" rather than having to apply an order value on the item, you might be able to improve performance.

{
    "source1": [
        {"id": "somevalue"},
        {"id": "someothervalue"},
        ...
    ],
    "source2": [
        ...
    ],
    ...
}

Some partially-implemented code for this approach might look like:

var output = {};
for(var i = 0, len = data.length; i < len; i++) {
    var el = data[i];
    var group = el.source;
    var insert = { 'id': el.id };
    if(group in output) {
        // perform sorted insert into array at group
        sortedInsert(output[group], insert);
    } else {
        output[group] = [ insert ];
    }
}

Note I say "partially" implemented based on my not having implemented the mechanism for sorted insert into group array.  You can certainly search online for an optimized approach for performing a sorted insert into an array which might use insertion sort, quicksort, heapsort, or some combination of those depending on expected array sizes.

This approach has you iterating the input data array only a single time, but with each pass you would be performing an increasingly complex insert which could range from \$O(1)\$ to \$O(n log(n))\$ (if appropriate sorting logic is used).

This could lead to worst case \$O(n^2 log(n))\$ operation. This my sound bad but consider that if you had X groups in your data set each of roughly similar size such that typical group size m = n / X that average case could actually could improve significantly:

\$O(n * m log(m)) \rightarrow O(n * (\frac{n}{X}) log (\frac{n}{x}))\$

For "worst case" example list of 1000 items in single group:

\$O(1000 * 1000 * log(1000)) \rightarrow O(3000000)\$

vs. better example of 1000 items in 10 equal groups:

\$O(1000 * 100 * log(100)) \rightarrow O(200000)\$

vs. first example of sort + mapping, again with 1000 items (number of groups doesn't matter):

\$O(1000^2 + 1000) \rightarrow O(1001000)\$

With the second approach, if you needed to, you could then "flatten" the data structure to get to your desired output, an operation that would cost an additional \$O(n)\$. If would be \$O(201000)\$ if we extended the 1000 items in 10 equal groups example.

You can see that the value of n that you expect for your use cases and how items are distributed amongst groups may become important to understanding which approach might be optimal for your application.

This leads to the most important point of this whole post, which is that if you are truly trying to optimize performance, you may need to potentially attempt several different implementations and then actually test them against your expected typical and "worst case" data sets to understand what sort of performance you might actually expect from this piece of code.


Is this method a good candidate for a service layer? Should the following GetPhysicalFileLocation() method in the Car class have its own Service Layer?

public class Car
{
    public int id;
    public string model;
    public int year;
    public IList<Pic> lstCarPics;
}

Then we have the respective class:

public class Pic
{
    public string filename;
    public string fileLocation;
    public string GetPhysicalFileLocation()
    {
        string LocalDirectory = "~\\CarFiles\\";
        DirectoryInfo diPath = new
            DirectoryInfo(HttpContext.Current.Server
                         .MapPath(LocalDirectory + car.id);
    }
}

Seeing that this method and others will access physical file locations throughout the application, would something like this justify having it's own FileService?

I'm having trouble understanding when methods like this should fall under it's own service layer vs leaving it in its class.

As @Vogel612 hinted, a class shouldn't expose its fields publicly - this:

public class Car
{
    public int Id { get; set; }

    // ...
}

Is equivalent to this:

public class Car
{
    private int _id;
    public int Id
    {
        get { return _id; }
        set { _id = value; }
    }

    // ...
}

As you can see, encapsulation is easily achieved with auto-properties - don't expose your fields!

Method GetPhysicalFileLocation has dependencies on the file system and on an HttpContext, so you definitely want that to be abstracted away, so you can write a test that uses that code and doesn't actually hit the file system.

I'd move it to some IFileSystemService implementation:

public interface IFileSystemService
{
    string GetPhysicalFileLocation(int carId);

    // ...
}

And then make a class that implements it:

public class FileSystemService : IFileSystemService
{
    // load this value from application settings?
    private readonly string _localDirectory = "~\\CarFiles\\";

    public string GetPhysicalFileLocation(int carId)
    {
        var diPath = new DirectoryInfo(HttpContext.Current
                                                  .Server.MapPath(_localDirectory + carId);
    }
}

And now you can have client code like this:

public class MyClass
{
    private readonly IFileSystemService _service;

    public MyClass(IFileSystemService service)
    {
        _service = service;
    }

    public void DoSomething(int carId)
    {
        var fileName = _service.GetPhysicalFileLocation(carId);
        // ...
    }
}

This way you can inject a "fake"/"mock" implementation for IFileSystemService and test DoSomething without hitting the file system or depending on an HttpContext; the key being that depending on the file system and on an HttpContext, is an implementation detail that the client code doesn't need to know about.


Splitting an NSArray into an NSDictionary of array more elegantly I need to split one NSArray into NSDictionary. Every key in NSDictionary will contain an NSArray with the object with the same value.

i.e. I have an array with 1000 customers and I want create an NSDictionary based on their zip code.

I wrote this code into an NSArray category and it works, but I'm looking for a better name and a way (if it exists) to do the same job with the KVC.

-(NSDictionary *)groupArrayWithBlock:(id<NSCopying> (^)(id obj))block {
    NSMutableDictionary *dictionary = [NSMutableDictionary dictionary];

    for (id obj in self) {
        id<NSCopying> key = block(obj);
        if (! dictionary[key]) {
            NSMutableArray *arr = [NSMutableArray array];
            dictionary[key] = arr;
        }
        [dictionary[key] addObject:obj];
    }
    return [dictionary copy];
}

As far as name of the method is concerned I have two points:

There is no need of the work Array here, as it is NSArray instance method.
The work With gives wrong impression here. (the phrase "group a with b" will generally mean to group them together).

So in my opinion groupUsingBlock or dictionaryGroupedUsingBlock, might be better.

Regarding KVC, if grouping is required to be done on a single property and that property is in complaint with the standard, you can have the function as following:

 -(NSDictionary *)groupByKey:(NSString *) key {
    NSMutableDictionary *dictionary = [NSMutableDictionary dictionary];

    for (id obj in self) {
        id keyValue = [obj valueForKey:key];
        NSMutableArray *arr = dictionary[keyValue];
        if (! arr) {
            arr = [NSMutableArray array];
            dictionary[keyValue] = arr;
        }
        [arr addObject:obj];
    }
    return [dictionary copy];
}

This will make the method slightly easy to use, but will also seriously limit the flexibility. So I would suggest that you keep the method which uses Block and implement the KVC version of the method by using it, like following:

 -(NSDictionary *)groupByKey:(NSString *) key {
    return [self groupUsingBlock:^(id obj) {
            return [obj valueForKey:key];
        }];
}


Laravel makeHidden (how to use) I am using Laravel 5.3, and currently working on models.

I have the following code:

$result = TheEpisode::find($id)->with('TheParts')->take(1)->get()->each(function ($item) {
      $item->TheParts->makeHidden("title");
});

The purpose of this code is to display all the data from TheEpisode and TheParts, but exclude the title from there.

Can this be improved, like not using a loop to exclude the title attribute?

no need to loop as you are using find method and find method always give single result

$result = TheEpisode::find($id);
$result->TheParts->makeHidden("title");
dd($result->toArray());


Detecting Windows Phone and Tablet clients This is called from a VF page,

styleClass="{!IF(isWindows8Device, 'windows', 'not_windows')}"

So if it's a windows device, return true.

The best thing to do would be to add one method in DeviceDetectionCls to check for any windows device, instead of two methods.

For the purpose of this question though, I've to use both.

Could it be improved at all?

public Boolean isWindows8Device{
    get{
         String userAgent = System.currentPageReference().getHeaders().get('User-Agent');

         if(DeviceDetectionCls.isWindows8Phone(userAgent)){
            return true;
         }

         if(DeviceDetectionCls.isWindows8Tablet(userAgent)){
            return true;
         }

         return false;
    }
}

I guess something like :

return DeviceDetectionCls.isWindows8Phone(userAgent) || DeviceDetectionCls.isWindows8Tablet(userAgent);

would do the trick.


Reflection optimization In a web application I'm working on I have a class that Map objects to other (different) objects. It is implemented with something like:

public interface IMapper
{
    object Map<T>(T item);
}

public class Mapper
{
    private static ConcurrentDictionary<Type, Delegate> cache = new ConcurrentDictionary<Type, Delegate>();

    public Mapper()
    {
        // dependencies are injected in the constructor
        // not shown here for simplicity
    }

    public object Map<T>(T item)
    {
        if (cache.ContainsKey(typeof(T)))
            return (cache[typeof(T)] as Func<T, object>)(item);

        var method = typeof(Mapper).GetMethod("Map", BindingFlags.Public | BindingFlags.Instance, null, new[] { typeof(T) }, null);

        var @delegate = (Func<T, object>)Delegate.CreateDelegate(typeof(Func<T, object>), this, method);
        cache.TryAdd(typeof(T), @delegate);

        return @delegate(item);

        // I'm trying not to use this
        //return method.Invoke(this, new object[] { item });
    }

    public object Map(Guid item) { return new object(); }
}

In order to assess what I believe should be a performance gain (compared to the .Invoke() solution) I tried to generate ten million objects (tested with strings) and then I ran the mapper for all of them and printed out the time elapsed. The results are.

Direct call: 1.67 s
Reflection: 14.09 s
Optimized Reflection: 2.62 s

which is not bad actually but I wonder if I'm testing this correctly.

Test code

 var guidList = new List<Guid>();
        for(int i = 0; i < 10000000; i++)
        {
            guidList.Add(Guid.NewGuid());
        }

        var mapper = new Mapper();

        var sw = new Stopwatch();
        sw.Start();
        var result = guidList.Select(_ => mapper.Map<Guid>(_)).ToList();
        sw.Stop();

(Obviously, when I tested the direct method call I changed the select lambda expression to _ => mapper.Map(_))

Am I doing it right?

On a side note: I declared the ConcurrentDictionary as ConcurrentDictionary<Type, Delegate> and so I have to "cast" the delegate part like this (cache[typeof(T)] as Func<T, object>)(item) when I'm using it. Is there a better way to do this?

Instead of using a ConcurrentDictionary as a cache, and having to cast, you can use an embedded generic static class. The static construction is lazy and threadsafe by design. To make it work you have to create and open delegate (an instance method with no "this") for the mapper function:

public class Mapper
{
    public Mapper()
    {
        // dependencies are injected in the constructor
        // not shown here for simplicity
    }

    public object Map<T>(T item)
    {
        var mapper = MapperCache<T>.Map;
        return mapper(this, item);
    }

    // example methods
    public object Map(SomeObject item) { return new object(); }

    public object Map(SomeTotallyDifferentObject item) { return new object(); }

    public object Map(Guid item) { return new object(); }

    private static class MapperCache<T>
    {
        public static readonly Func<Mapper, T, object> Map;

        static MapperCache()
        {
            var mapMethod = typeof(Mapper).GetMethod("Map", BindingFlags.Public | BindingFlags.Instance, null, new[] { typeof(T) }, null);

                Map = mapMethod != null ? (Func<Mapper, T, object>)Delegate.CreateDelegate(typeof(Func<Mapper, T, object>), mapMethod)
                                        : (mapper, item) => { throw new InvalidOperationException($"{nameof(Mapper)} cannot map from {typeof(T).Name} to object"); };
        }
    }
}

The first time you reference the static property, Map, the MapperCache<T> for the given type T will be initialized. By setting up the delegate creation in the static constructor you guarantee that it will only run once for each T. If any exceptions are thrown in the static constructor the type will fail to load and cannot be used at runtime.

The same trick is used in System.Data.DataRowExtensions (source) to cache converters.


Bear and Candies Game I am trying to improving my Java skills and I've been doing various different projects/challenges which I usually am able to complete. I unfortunately never have my code reviewed so while my code may work, I would love to hear ways to do things better. I went on Codechef and solved one of the problems and added some basic error handling in addition to what the problem asked. I tried to write the code as clearly as possible to avoid unnecessary comments.

Main.java

import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;

public class Main {

static List<Integer> maxEatingCaps_Limark = new ArrayList<>();
static List<Integer> maxEatingCaps_Bob = new ArrayList<>();

public static void main(String[] args) {

    setUp();
    Bear Limak = new Bear(maxEatingCaps_Limark,"Limak");
    Bear Bob = new Bear(maxEatingCaps_Bob,"Bob");
    Bear.startEating(Limak,Bob);

    }

public static void setUp() {
    int  numOfTestCases = 0;
    Scanner input = new Scanner(System.in);
    System.out.println("Enter number of test cases");

    try {
        numOfTestCases = Integer.parseInt(input.nextLine().trim());
    }
    catch (NumberFormatException e) {
        System.err.println("You must enter an number" );
        System.exit(1);
    }

    boolean validInput = false;
    for (int i = 0; i < numOfTestCases ; i++) {
        validInput = false;

        while (!validInput)
            try {

                String maxEatingInput = input.nextLine();
                String[] individualEatingCaps = maxEatingInput.trim().split("\\s+");
                maxEatingCaps_Limark.add(Integer.parseInt(individualEatingCaps[0]));
                maxEatingCaps_Bob.add(Integer.parseInt(individualEatingCaps[1]));
                validInput = true;

            } catch (NumberFormatException | ArrayIndexOutOfBoundsException e) {
                System.err.println("Enter in the form: INTEGER:(Limark's Max Eating Capacity) INTEGER:(Bob's Max Eating Capacity)");
                validInput = false;

            }

    }

}

Bear.java

import java.util.List;

public class Bear {

    private static int amountOfCandyToEat;
    private static boolean haveWinner;
    private static String currentLeader;
    private int candyConsumed;
    private List<Integer> maxEatingCapacity;
    private String name;
    private static int numOfTest;

    Bear(List<Integer> maxEatingCapacity, String name) {

        this.maxEatingCapacity = maxEatingCapacity;
        this.name = name;
        candyConsumed = 0;
        numOfTest = maxEatingCapacity.size();
    }

  public static void  startEating(Bear bear1, Bear bear2) {

        for (int i = 0; i < numOfTest ; i++) {

            amountOfCandyToEat = 1;
            haveWinner = false;
            currentLeader = null;
            bear1.reset();
            bear2.reset();

            while(!haveWinner) {

                tryEating(bear1,i);
                tryEating(bear2,i);
            }
            System.out.println(currentLeader);

        }
        }
    private void reset() {
        candyConsumed = 0;
    }

    private static void tryEating(Bear bear, int index) {

        if (bear.getCandyConsumed() + Bear.getAmountOfCandyToEat() <= bear.getMaxEatingCapacity(index)) {
            bear.setCandyConsumed(bear.getCandyConsumed() + Bear.getAmountOfCandyToEat());
            Bear.setCurrentLeader(bear.getName());
            amountOfCandyToEat++;
        }
        else{
            haveWinner = true;
        }
    }

  private String getName() {
      return name;
  }

  private static int getAmountOfCandyToEat() {
      return amountOfCandyToEat;
  }

  private int getMaxEatingCapacity(int index) {
      return maxEatingCapacity.get(index);
  }

  private int getCandyConsumed() {
      return candyConsumed;
  }

  private void setCandyConsumed(int candyConsumed) {
      this.candyConsumed = candyConsumed;
  }

  private static void setCurrentLeader(String currentLeader) {
      Bear.currentLeader = currentLeader;
  }
}

Helper methods

    int  numOfTestCases = 0;
    Scanner input = new Scanner(System.in);
    System.out.println("Enter number of test cases");

    try {
        numOfTestCases = Integer.parseInt(input.nextLine().trim());
    }
    catch (NumberFormatException e) {
        System.err.println("You must enter an number" );
        System.exit(1);
    }

You can rewrite this as

    Scanner input = new Scanner(System.in);
    System.out.println("Enter number of test cases");

    int remaining = inputInteger(input);
    input.nextLine();

With

public static void inputInteger(Scanner input) {
    try {
        return input.nextInt();
    } catch (NumberFormatException e) {
        System.err.println("You must enter a number" );
        System.exit(1);
    }
}

This saves having to separate declaration and initialization of the variable.  Now you can do both in one line.

I prefer the name remaining to numOfTestCases.  Why may become clearer when you see how I use it.

You don't need to jump through hoops to get an integer with a Scanner.  There's a method just for that.

Don't overcomplicate things

    boolean validInput = false;
    for (int i = 0; i < numOfTestCases ; i++) {
        validInput = false;

        while (!validInput)
            try {

                String maxEatingInput = input.nextLine();
                String[] individualEatingCaps = maxEatingInput.trim().split("\\s+");
                maxEatingCaps_Limark.add(Integer.parseInt(individualEatingCaps[0]));
                maxEatingCaps_Bob.add(Integer.parseInt(individualEatingCaps[1]));
                validInput = true;

            } catch (NumberFormatException | ArrayIndexOutOfBoundsException e) {
                System.err.println("Enter in the form: INTEGER:(Limark's Max Eating Capacity) INTEGER:(Bob's Max Eating Capacity)");
                validInput = false;

            }

    }

}

You don't need validInput.  Consider

    while (remaining > 0) {
        try {
            String maxEatingInput = input.nextLine();
            String[] individualEatingCaps = maxEatingInput.trim().split("\\s+");
            maxEatingCaps_Limark.add(Integer.parseInt(individualEatingCaps[0]));
            maxEatingCaps_Bob.add(Integer.parseInt(individualEatingCaps[1]));
            remaining--;
        } catch (NumberFormatException | ArrayIndexOutOfBoundsException e) {
            System.err.println("Enter in the form: INTEGER:(Limark's Max Eating Capacity) INTEGER:(Bob's Max Eating Capacity)");
        }
    }

This saves you two variables and a loop and gives the same result.

Note that you could also drop down to

    while (remaining > 0) {
        try {
            maxEatingCaps_Limark.add(input.nextInt());
            maxEatingCaps_Bob.add(input.nextInt());
            remaining--;
        } catch (NumberFormatException e) {
            System.err.println("Enter in the form: INTEGER:(Limark's Max Eating Capacity) INTEGER:(Bob's Max Eating Capacity)");
        }
    }

But this doesn't enforce the two numbers per line requirement.  Up to you if you care.

Equations over iteration

You don't have to iterate through the candy.  You can do the quadratic formula instead.

$$\frac{(n + 1)^2}{4} = m$$

Where \$n\$ is the number of candies that first bear eats and \$m\$ is the maximum for that bear.  Solving for \$n\$ gives

$$n = 2\sqrt{m} - 1$$

Rounding down to an odd integer gives the last number of candies that bear can eat. Add 2 to give the first number that it can't eat.  Now do the equivalent for the second bear.

$$n(n+1) = m$$

And the quadratic formula gives

$$n = \frac{-1 \pm \sqrt{1 + 4m}}{2}$$

Negative numbers aren't meaningful here, so

$$n = \frac{\sqrt{4m + 1} - 1}{2}$$

Round down to an even integer to get the last number of candies that can be eaten.

Whoever gets farther wins.

Now you just have to turn those equations into Java.


IPtables for webserver with SSH I've had a bit of help but I'm still a bit unsure. I have an nginx webserver with SSH only required. I wrote this rule set based on information I found on the internet:

i=/sbin/iptables

# Flush all rules
$i -F
$i -X

# Setup default filter policy
$i -P INPUT DROP
$i -P OUTPUT DROP
$i -P FORWARD DROP

# DONT KNOW WHAT THESE DO
$i -A INPUT -i lo -j ACCEPT
$i -A INPUT -p icmp --icmp-type any -j ACCEPT

# Force SYN checks
$i -A INPUT -p tcp ! --syn -m state --state NEW -j DROP

# Drop all fragments
$i -A INPUT -f -j DROP

# Drop XMAS packets
$i -A INPUT -p tcp --tcp-flags ALL ALL -j DROP

# Drop NULL packets
$i -A INPUT -p tcp --tcp-flags ALL NONE -j DROP

# Allow established connections
$i -A INPUT -m state --state ESTABLISHED,RELATED -j ACCEPT

# Allow unlimited traffic on loopback
$i -A INPUT -i lo -j ACCEPT
$i -A OUTPUT -o lo -j ACCEPT

# Open nginx
$i -A INPUT -p tcp --dport 443 -j ACCEPT
$i -A INPUT -p tcp --dport 80 -j ACCEPT

# Open SSH
$i -A INPUT -p tcp --dport 22 -j ACCEPT

Can anyone please tell me if I am taking the right approach here, and if I can improve it in any way?

Scripts should have a shebang line. If you happen to be running in a shell other than 'sh' or compoatible, then this script will fail. Add:

#!/bin/sh

as the very first line.

The rest of the script looks fine enough.

The script has a duplicate declaration:

$i -A INPUT -i lo -j ACCEPT

That line says that, if the computer is talking to itself (on the lo interface , the 'localhost'...), that it is allowed to listen to itself.

The other line:

$i -A INPUT -p icmp --icmp-type any -j ACCEPT

says that the computer will listen and respond to 'ping' requests.

The computer will also allow the two standard webserver ports (http and https) but that assumes your webserver is running on those ports.

Also allowed is ssh (port 22).

In essence, the script is doing what you need. You have a server that can listen on the standard http, https and ssh ports only on the public interfaces. You can also ping it.

What is unusual is the very restrictive outbound rules. This is nice, if you can live with it, but, for example, your computer will not be able to connect to anything else, so, it cannot use:

NTP
DNS
send mail
ping other computers
ssh out
etc.

It is recommended practice to have a default-DROP policy for OUTPUT, but, in practice, I have seen very few servers that actually do that.... it is hard to live with.


Generic sliding average in C++ I tried hard to come up with a function template computing a sliding average. A sliding average over data \$x_1, x_2, \dots, x_n\$ with window length \$k \leq n\$ is a sequence \$y_1, y_2, \dots, y_{n - k + 1}\$, where
$$y_i = \frac{1}{k}\sum_{j = 0}^{k - 1} x_{i + j}.$$

Here is my attempt:

mymath.h

#ifndef MYMATH_H
#define MYMATH_H

#include <iterator>
#include <sstream>
#include <stdexcept>

template<typename ForwardIterator, typename OutputIterator>
void sliding_average(ForwardIterator begin,
                     ForwardIterator end,
                     OutputIterator output,
                     size_t window_length)
{
    if (window_length == 0)
    {
        std::stringstream ss;
        ss << "Bad window_length: ";
        ss << window_length;
        throw std::runtime_error{ss.str()};
    }

    using T = typename std::iterator_traits<ForwardIterator>::value_type;

    ForwardIterator finger = begin;
    T sum {};
    size_t count = 0;

    while (finger != end and count < window_length)
    {
        sum += *finger++;
        count++;
    }

    if (count < window_length)
    {
        std::stringstream ss;
        ss << "The length of the range (";
        ss << count;
        ss << ") is too short. Must be at least ";
        ss << window_length;
        throw std::runtime_error{ss.str()};
    }

    *output++ = sum / window_length;
    ForwardIterator window_tail = begin;

    while (finger != end)
    {
        sum -= *window_tail++;
        sum += *finger++;
        *output++ = sum / window_length;
    }
}

#endif // MYMATH_H

main.cpp

#include "mymath.h"
#include <iostream>
#include <iterator>

using std::cout;
using std::endl;
using std::begin;
using std::end;

int main(int argc, const char * argv[]) {
    float input[15];

    for (size_t i = 0; i < 15; ++i)
    {
        input[i] = i + 1;
    }

    float output[11];

    sliding_average(begin(input), end(input), begin(output), 5);

    for (auto& a : output)
    {
        cout << a << " ";
    }

    cout << endl;
    return 0;
}

Critique request

I would like to receive comments regarding how to make my implementation more generic, and how to make it more idiomatic. Other comments are welcome as well.

Very good. There are small things that could be improved.

It is possible to give window_length type std::iterator_traits<ForwardIterator>::difference_type. I think std::size_t is fine for most cases.

ForwardIterator begin,
ForwardIterator end

Usually those are called first and last.

if (window_length == 0)
{
    std::stringstream ss;
    ss << "Bad window_length: ";
    ss << window_length;
    throw std::runtime_error{ss.str()};
}

Well, the result of ss.str() is obvious :) It is possible to write constexpr there, or throw the string right into the constructor. Also, runtime_error is a good fit, but it has child called invalid_argument, which perfectly matches the case.

if (count < window_length)
{
    std::stringstream ss;
    ss << "The length of the range (";
    ss << count;
    ss << ") is too short. Must be at least ";
    ss << window_length;
    throw std::runtime_error{ss.str()};
}

I think that using std::stringstream is an overkill here. Throwing just "The length of the range is too short. It must be at least of length window_length" is pretty good by itself, since most IDEs will probably stop execution, so that programmers could have a look. Even if they had a catch for this, they would need to parse a string to be actually able to do something. I don't think it worth the troubles it brings.

Some caveats:

Currently if T = int the algorithm is going to produce somewhat incorrect results. May be you could write something like warning mechanism that will warn when integer type is used. I would consider #pragma message("your warning message here"). It might get portability problems but the code will still compile since unrecognized #pragmas are ignored.


Primality test using recursion For the given below exercise:

  Here is one method to check if a number is prime:

def is_prime(n):
    k = 2
    while k < n:
        if n % k == 0:
            return False
        k += 1
    return True

  How does this function work?

  This is a decent way of testing if a number is prime, but looping k all the way to n
  might be a bit cumbersome. As a little bonus question, can you think of a better place
  to stop?

  Using the is_prime function, fill in the following procedure, which generates the nth
  prime number. For example, the 2nd prime number is 3, the 5th prime number is 11,
  and so on.

def nth_prime(n):

Below is the solution:

def is_prime(n):
    def f(n, k):
        if k > (n // 2):
            return True
        elif n % k == 0:
            return False
        else:
            return f(n, k + 1)
    return f(n, 2)

def nth_prime(n):
    def traverse_for_nth_prime(count, k):
        if count == n:
           return k-1
        elif is_prime(k):
            return traverse_for_nth_prime(count + 1, k + 1)
        else:
            return traverse_for_nth_prime(count, k + 1)
    return traverse_for_nth_prime(0, 2)

print(nth_prime(20))

Above code is written in functional paradigm. Can it be improved, especially in relation to being optimised from recursion depth limit perspective, by strictly following functional paradigm?

A "functional" method avoiding recusion would be

from itertools import count

def gen_primes():
    return filter(is_prime, count(2))

def nth(n, iterable):
    return next(val for i, val in enumerate(iterable) if i == n)

def nth_prime(n):
    return nth(n, gen_primes())

To prove that this is functional, here's the Haskell:

enumerate = zip [0..]

primes = filter is_prime [2..]

nth iterable n = head [v | (i, v) <- enumerate iterable, i == n]

nth_prime = nth primes

The trick is to make everything as small as logically possible and see how to extract common functionality.


Improving design for a CustomerDAO with DatabaseManager object I'm learning about SRP and had to reduce the responsibility of a customer object, which contained too much info. So I turned that into just a Pojo and pulled out the database logic.
I'm trying to find the best design for this setup and am split between making the DatabaseManager class be a parent class, or a singleton that can return the single connection object. This system will be used to insert and delete customers from the database. I don't want my domain objects/DAOs to worry about the connection details. Is the design I currently have a strong follower of OOP design principles?

public class DatabaseManager {

    private Connection conn;

    private static DatabaseManager managerInstance = new DatabaseManager();

    private DatabaseManager() {

    }

    public static DatabaseManager getInstance() {
        return managerInstance;
    }

    /**
     * contains connection details
     *
     * @throws SQLException
     */
    public void connect() throws SQLException {
        System.out.println("Established Database Connection...");
        conn = DriverManager.getConnection("Some/Database/URL");
    }

    public Connection getConnectionObject() {
        return conn;
    }

    public void disconnect() throws SQLException {
        conn.close();
        System.out.println("Disconnected from Database...");
    }
}

Here is the Customer Object:

public class Customer {

       private int id;
       private String name;
       private boolean active;

       public Customer(int id, String name, String department, boolean working) {
              super();
              this.id = id;
              this.name = name;
              this.department = department;
              this.working = working;
       }

       @Override
       public String toString() {
              return "Customer [id=" + id + ", name=" + name + ", department="
                           + department + ", working=" + working + "]";
       }
}

The customer DAO:

public class CustomerDAO {

    public CustomerDAO() {
    }

    public void addCustomer(Customer Customer) throws SQLException {
        DatabaseManager.getInstance().getConnectionObject().prepareStatement("some sql... ");
    }

    public void removeCustomer(Customer Customer) throws SQLException {
        DatabaseManager.getInstance().getConnectionObject().prepareStatement("some sql... ");
        // implementation details avoided
    }

}

Single responsibility principle

In terms of SRP, the current code is good, each class is has one clear purpose:

Customer: only the customer details, nothing else
CustomerDAO: only data accessor methods related to customers
DatabaseManager: manage database connection, DatabaseConnectionManager might be a better name

But they can be improved.

Customer

If you can make its fields final, make them final.

No need to call super() in a class whose parent is Object. You can just delete that line.

CustomerDAO

The throws SQLException declarations make this class database specific. In theory, a CustomerDAO could work over a web service or a CSV file. Actually CustomerDAO should be an interface, to model a backend-agnostic concept, with throws declarations that are backend-agnostic. Then an implementation using a database backend should wrap the SQLExceptions and throw the backend-agnostic exceptions instead. In such design you will have the flexibility to replace the DAO with a different backend.

Also, the public empty constructor is pointless.

DatabaseManager

The modern pattern of implementing singletons is this:

public enum DatabaseManager {

    INSTANCE;

    // ...
}

This is truly thread-safe. Use this instead of a private static field, which is not truly thread-safe.

Use a framework

Instead of managing database connections yourself, I strongly recommend using a framework that can greatly simply it for you. For example Spring Framework.


Simple medication dosage calculator I am a nurse and I'm learning to code. I have coded this simple tool that helps me doing some common nursing formulas for medication. It works, however I think it is too bulky/redundant. Can you help me with some ideas for refactoring it?

class NursingFormulas
  def initialize
    puts "What do you want to do?"
    puts "1) How many tablets should I administer?"
    puts "2) How many ml should I administer?"
    puts "3) What's the ml per hour rate of my IV?"
    puts "4) What's the drop rate of my IV?"
    puts "5) Exit"
    choice = gets.chomp.to_i
    case choice
    when 1 then mass_for_mass
    when 2 then mass_for_liquid
    when 3 then iv_ml_rate
    when 4 then iv_drop_rate
    when 5 then exit(0)
    else
      exit(0)
    end
  end

  def mass_for_mass
    puts "What's the prescribed mass in mg?"
    prescribed = gets.chomp.to_f
    puts "What's the mass you've got in mg?"
    mass = gets.chomp.to_f
    result = prescribed / mass
    puts "Administer #{result} tablets"
  end

  def mass_for_liquid
    puts "What's the prescribed mass in mg?"
    prescribed = gets.chomp.to_f
    puts "What's the mass you've got in mg?"
    mass = gets.chomp.to_f
    puts "How many mL do you have?"
    ml = gets.chomp.to_f
    result = (prescribed / mass) * ml
    puts "Administer #{result} ml"
  end

  def iv_ml_rate
    puts "What's the volume in mL?"
    volume = gets.chomp.to_f
    puts "What's the time in hours?"
    time = gets.chomp.to_f
    result = (volume / time).round
    puts "The rate is #{result} ml per hour"
  end

  def iv_drop_rate
    puts "What's the volume in mL?"
    volume = gets.chomp.to_f
    puts "What's the time in hours?"
    time = gets.chomp.to_f
    time = time * 60
    puts "What's the drop factor?"
    factor = gets.chomp.to_f
    result = ((volume / time) * factor).round
    puts "The drop rate is #{result} drops per minute"
  end
end

Here is a solution using the Command Pattern.

Each action will live in its own class. I'm using some mixins so we don't have repeated code.

nursing_formulas.rb

require_relative "formulas/mass_for_mass"
require_relative "formulas/mass_for_liquid"
require_relative "formulas/iv_ml_rate"
require_relative "formulas/iv_drop_rate"

class NursingFormulas
  def initialize
    show_instructions

    @instructions = {
      1 => MassForMass.new,
      2 => MassForLiquid.new,
      3 => IvMlRate.new,
      4 => IvDropRate.new
    }

    choice = gets.chomp.to_i

    @instructions.key?(choice) ? @instructions[choice].do : exit(0)
  end

  def show_instructions
    puts "What do you want to do?"
    puts "1) How many tablets should I administer?"
    puts "2) How many ml should I administer?"
    puts "3) What's the ml per hour rate of my IV?"
    puts "4) What's the drop rate of my IV?"
    puts "5) Exit"
  end
end

mass_for_mass.rb

require_relative "mixins/mass_actions"

class MassForMass
  include MassActions

  def do
    result = ask_for_prescribed_mass / ask_for_mass
    puts "Administer #{result} tablets"
  end
end

mass_for_liquid.rb

require_relative "mixins/mass_actions"

class MassForLiquid
  include MassActions

  def do
    prescribed = ask_for_prescribed_mass
    mass = ask_for_mass
    ml = ask_for_ml

    result = (prescribed / mass) * ml
    puts "Administer #{result} ml"
  end

  def ask_for_ml
    puts "How many mL do you have?"
    gets.chomp.to_f
  end
end

iv_ml_rate.rb

require_relative "mixins/iv_actions"

class IvMlRate
  include IvActions

  def do
    volume = ask_for_volume
    time = ask_for_time

    result = (volume / time).round
    puts "The rate is #{result} ml per hour"
  end
end

iv_drop_rate.rb

require_relative "mixins/iv_actions"

class IvDropRate
  include IvActions

  def do
    volume = ask_for_volume
    time_in_min = ask_for_time * 60
    factor = ask_for_drop_factor

    result = ((volume / time_in_min) * factor).round
    puts "The drop rate is #{result} drops per minute"
  end

  def ask_for_drop_factor
    puts "What's the drop factor?"
    gets.chomp.to_f
  end
end

MIXINS

mass_actions.rb

module MassActions
  def ask_for_prescribed_mass
    puts "What's the prescribed mass in mg?"
    gets.chomp.to_f
  end

  def ask_for_mass
    puts "What's the mass you've got in mg?"
    gets.chomp.to_f
  end
end

iv_actions.rb

module IvActions
  def ask_for_volume
    puts "What's the volume in mL?"
    gets.chomp.to_f
  end

  def ask_for_time
    puts "What's the time in hours?"
    gets.chomp.to_f
  end
end


Insert value into multiple locations in a list Code
insertAt :: a -> [Int] -> [a] -> [a]
insertAt x indices xs = go 0 xs
  where
    go _ [] = []
    go j (y:ys)
      | nextIndex `elem` indices = y : x : go (nextIndex + 1) ys
      | otherwise = y : go nextIndex ys
      where
        nextIndex = j + 1

Questions

How can I make this code more efficient?
How can I make the code easier to follow?
Is a recursive approach the correct one?
Do the Prelude or Data.List have any functions that could make this more readable/efficient?

Is it intended that insertAt 999 [1] [1..10] inserts 999 at index 1, but insertAt 999 [0] [1..10] does nothing.  Shouldn't it insert a value at index 0?
Also, it looks like certain other insertions don't work correctly:
> insertAt 999 [2,5,6,7] [1..10]
[1,2,999,3,4,999,5,999,6,7,8,9,10]

Note that this inserts only at indices 2, 5, and 7 and appears to skip index 6.
My guess is that you probably wanted something more like:
insertAt :: a -> [Int] -> [a] -> [a]
insertAt x indices xs = go 0 xs
  where
    go _ [] = []
    go j (y:ys)
      | j `elem` indices = x : go (j+1) (y:ys)
      | otherwise        = y : go (j+1) ys

which handles these cases correctly:
λ> insertAt 999 [0] [1..10]
[999,1,2,3,4,5,6,7,8,9,10]
λ> insertAt 999 [2,5,6,7] [1..10]
[1,2,999,3,4,999,999,999,5,6,7,8,9,10]

It doesn't insert anything for insertAt 999 [10] [1..10], but if you want that to insert at the end, then you need a slight modification:
insertAt :: a -> [Int] -> [a] -> [a]
insertAt x indices xs = go 0 xs
  where
    go j yall   | j `elem` indices =  x : go (j+1) yall
    go j (y:ys)                    =  y : go (j+1) ys
    go _ []                        = []

which works like so:
λ> insertAt 999 [0] [1..10]
[999,1,2,3,4,5,6,7,8,9,10]
λ> insertAt 999 [2,5,6,7] [1..10]
[1,2,999,3,4,999,999,999,5,6,7,8,9,10]
λ> insertAt 999 [10] [1..10]
[1,2,3,4,5,6,7,8,9,10,999]
λ> insertAt 999 [10,11,12] [1..10]  -- multiple consecutive indices off the end
[1,2,3,4,5,6,7,8,9,10,999,999,999]

I think this version is quite readable.  The main efficiency problem is that, if indices is more than a few elements, the repeated linear traversals of the elem function through the indices list will be slow.  A second efficiency problem is that if all the insertions are near the beginning, you will end up traversing the entire input list even after all the insertions have been performed.
You can improve the performance substantially by using an IntSet.  (A plain Set from Data.Set would work, too, but IntSets are even faster for sets of Ints.)
import qualified Data.IntSet as IntSet
insertAt :: a -> [Int] -> [a] -> [a]
insertAt x indices = go 0
  where
    indices' = IntSet.fromList indices
    go j yall | j `IntSet.member` indices' =  x : go (j+1) yall
    go j (y:ys)                            =  y : go (j+1) ys
    go _ []                                = []

You could also pass the IntSet as a parameter to go and delete indices as they are used.  This might be faster, but you'd need to benchmark it to be sure:
import qualified Data.IntSet as IntSet
insertAt :: a -> [Int] -> [a] -> [a]
insertAt x indices = go 0 (IntSet.fromList indices)
  where
    go j indices' yall | j `IntSet.member` indices' =  x : go (j+1) (IntSet.delete j indices') yall
    go j indices' (y:ys)                            =  y : go (j+1) indices' ys
    go _ _ []                                       = []

This would also allow you to stop inserting when the list of indices is empty, but again, only benchmarking will tell whether or not this is faster:
import qualified Data.IntSet as IntSet
insertAt :: a -> [Int] -> [a] -> [a]
insertAt x indices = go 0 (IntSet.fromList indices)
  where
    go _ indices' yall | IntSet.null indices'       =  yall
    go j indices' yall | j `IntSet.member` indices' =  x : go (j+1) (IntSet.delete j indices') yall
    go j indices' (y:ys)                            =  y : go (j+1) indices' ys
    go _ _ []                                       = []

Personally, I think I probably would have approached it by writing a go function that operates on the current index and a sorted list of indices:
insertAt x indices = go 0 (sort indices)

This way, we can directly compare the current index with the next insertion point in the sorted list:
go j (i:is) (y:ys) | i == j = ...

This variant would look something like:
insertAt :: a -> [Int] -> [a] -> [a]
insertAt x indices = go 0 (sort indices)
  where
    go _ []     yall              = yall
    go j (i:idxs) yall   | j == i = x : go (j+1) idxs yall
                         | j >  i = go j idxs yall -- delete duplicate indices
    go j idxs     (y:ys)          = y : go (j+1) is ys
    go _ _        []              = []

This avoids those efficiency problems I mentioned earlier without having to switch to a different data structure.  Sorting the index list and consuming it as insertions are made will avoid any traversals through the index list, since you're only ever looking at the list's head.  Also, when the index list is empty, the go _ [] yall = yall case will immediately return the remaining list without further processing.
I'm not sure that there's any clever, non-recursive method that uses folds or other functions from Data.List and ends up being better than the above.  You can replace the recursion with unfoldr.  For example, given the version from above:
insertAt :: a -> [Int] -> [a] -> [a]
insertAt x indices xs = go 0 xs
  where
    go j yall   | j `elem` indices =  x : go (j+1) yall
    go j (y:ys)                    =  y : go (j+1) ys
    go _ []                        = []

you can rewrite this as:
insertAt :: a -> [Int] -> [a] -> [a]
insertAt x indices lst = unfoldr step (0, lst)
  where step (j, lst)  | j `elem` indices = Just (x, (j+1, lst))
        step (j, y:ys)                    = Just (y, (j+1, ys))
        step (_, [])                      = Nothing

and the other versions can be rewritten similarly.  This doesn't particularly improve readability, and I doubt it's more efficient.
Another completely different approach might be to intercalate the insertion value into a sequence of sublists generated from the indexes.  So, for example, given the indexes [2,5,6,7], you might want to first break the input list [1..10] up into sublists between the insertions:
[[1,2],[3,4],[],[],[5,6,7,8,9,10]]

It might be easiest to first convert a sorted list of indices into a list of sizes for all the sublists except the last:
indicesToChunks :: [Int] -> [Int]
indicesToChunks indices = zipWith (\i j -> i-j-1) indices (-1 : indices)

giving:
> indicesToChunks [2,5,6,7]
[2,2,0,0]

and then write a function to break a list into sublists based on these sublist sizes.  It's pretty easy to write recursively:
chunks :: [Int] -> [a] -> [[a]]
chunks [] lst = [lst]
chunks (n:ns) lst = a : chunks ns b
  where (a,b) = splitAt n lst

Then, you can write:
insertAt :: a -> [Int] -> [a] -> [a]
insertAt x indices = intercalate [x] . chunks (indicesToChunks (sortu indices))
  where -- unique elements in sorted order
        sortu = map head . group . sort

The definition of sortu is weird, but it's a reasonable method of sorting and removing duplicates.  (The nub function in Data.List removes duplicates, but doesn't do it efficiently for sorted lists, the way map head . group does.)
Note that this version behaves a little differently than the others, if there are insertions at the end of the list that skip indexes.  The earlier versions all insert only one 999 at the end of the list in this example:
> insertAt 999 [10,1000] [1..10]
[1,2,3,4,5,6,7,8,9,10,999]

treating the 1000 as an out-of-bounds index.  However, the intercalate-based version inserts two 999s:
> insertAt 999 [10,1000] [1..10]
[1,2,3,4,5,6,7,8,9,10,999,999]


Reload control using loop I created this working sample and the code is below. Any advice on how to make this code; cleaner, more effective, just overall better! The data is coming from an API and I need to parse, select the value in location 0 set the title and then rebuild the drop down control.

can it be done without the JSON.parse(data) or the $.each?

https://dotnetfiddle.net/lfJhGi
      $("#ddlReleaseYears").change(function () {
            $('#toptitle').text('change event');
            //coming from API
            var data = "[2021,2020,2019,2018,2017,2016]";
            var arr = JSON.parse(data);
            $('#toptitle').text(arr[0]);
            var HTML = ""
            $.each(arr, function (index, value) {
                 HTML += "<option value='" +value + "' ";
                 HTML += ">" +value + "</option>"
            });
            $("#ddlReleaseYears").html("");
            $("#ddlReleaseYears").append(HTML);
        })

The two first things that come to my mind:

Using template strings
Modifying the dropdown HTML once:

$(document).ready(function () {
    $("#ddlReleaseYears").change(function () {
        var data = "[2021,2020,2019,2018,2017,2016]";
        var arr = JSON.parse(data);
        $('#toptitle').text(arr[0]);
        var HTML = ""
        $.each(arr, function (index, value) {
            HTML += `<option value="${value}">${value}</option>`
        });
        // No need to set the html to "" and then append
        $("#ddlReleaseYears").html(HTML);
     })
});


Personal project for managing photos I have wrote a small project for Android. It's an app which lets you to take a photo (or find one with Flickr) and save it in the cloud (Cloudinary).

Link to the GitHub repository.

Should MVP layers be completely separated?

In the repository layer I have two DbPhoto classes (one for local database [SQLite] and one for cloud database [Firebase]), that are converted to Photo class in model. Photo class is used also in View. Should View has its own class?

DbPhoto for local database:

package org.chyla.photoapp.Repository.LocalDatabase.GreenDao.detail;

import org.greenrobot.greendao.DaoException;
import org.greenrobot.greendao.annotation.Entity;
import org.greenrobot.greendao.annotation.Generated;
import org.greenrobot.greendao.annotation.Id;
import org.greenrobot.greendao.annotation.Index;
import org.greenrobot.greendao.annotation.NotNull;
import org.greenrobot.greendao.annotation.ToOne;

@Entity(
        indexes = {
                @Index(value = "dbUserId,title,description,url", unique = true)
        }
)
public class DbPhoto {

    @Id
    private Long id;

    @NotNull
    private Long dbUserId;

    @ToOne(joinProperty = "dbUserId")
    private DbUser dbUser;

    @NotNull
    private String title;

    @NotNull
    private String description;

    @NotNull
    private String url;

/** Used to resolve relations */
@Generated(hash = 2040040024)
private transient DaoSession daoSession;

/** Used for active entity operations. */
@Generated(hash = 1256174471)
private transient DbPhotoDao myDao;

@Generated(hash = 1342142122)
public DbPhoto(Long id, @NotNull Long dbUserId, @NotNull String title,
        @NotNull String description, @NotNull String url) {
    this.id = id;
    this.dbUserId = dbUserId;
    this.title = title;
    this.description = description;
    this.url = url;
}

@Generated(hash = 95134667)
public DbPhoto() {
}

public Long getId() {
    return this.id;
}

public void setId(Long id) {
    this.id = id;
}

public String getTitle() {
    return this.title;
}

public void setTitle(String title) {
    this.title = title;
}

public String getDescription() {
    return this.description;
}

public void setDescription(String description) {
    this.description = description;
}

public String getUrl() {
    return this.url;
}

public void setUrl(String url) {
    this.url = url;
}

// ... (generated functions)

DbPhoto for cloud database:

package org.chyla.photoapp.Repository.CloudDatabase.Firebase.detail;

public class DbPhoto {

    public String title;
    public String description;
    public String url;

    public DbPhoto() {
    }

    public DbPhoto(String title, String description, String url) {
        this.title = title;
        this.description = description;
        this.url = url;
    }

}

Photo class for model and view:

package org.chyla.photoapp.Main.Model.objects;

import android.support.annotation.Nullable;

import java.net.URL;

public class Photo {

    private final String title;
    private final String description;
    private final URL url;

    public Photo(String title, @Nullable String description, URL url) {
        this.title = title;
        this.description = description;
        this.url = url;
    }

    public String getTitle() {
        return title;
    }

    public String getDescription() {
        if (description == null) {
            return "";
        }
        else {
            return description;
        }
    }

    public URL getUrl() {
        return url;
    }

}

I'm open to every suggestion improvements for my project.

Default description

public String getDescription() {
    if (description == null) {
        return "";
    }
    else {
        return description;
    }
}

Depending on how this method is used, it may make sense to take in a default value here, and return the default if the field is null, this may simplify code using this class.


Download Progress Calculator Here is my progress calculator class. It is used for downloads/uploads. It takes bytes in and calculates the percentage(to the nearest 5%) that should show on a progress bar. Please let me know how I could improve it. Thanks a lot!!

public class ProgressCalculator {

    private int totalAmount;
    private int progressAmount;
    private int amountProgressItems;
    private int progressBarPercentage;

    //returns if the progress bar needs to change
    public boolean progress(int amount) {
        progressAmount = progressAmount + amount;
        return updateProgressbarPercentage();
    }

    public int getCurrentValue() {
        return progressBarPercentage;
    }

    //returns if the progressbar needs to change
    public boolean addProgressItem(int itemAmount) {
        totalAmount = totalAmount + itemAmount;
        amountProgressItems++;
        return updateProgressbarPercentage();
    }

    //returns true if has been updated;
    private boolean updateProgressbarPercentage() {
        int newProgressBarPercentage = round(calculateProgressPercentage());
        if (progressBarPercentage == newProgressBarPercentage) {
            return false;
        } else {
            progressBarPercentage = newProgressBarPercentage;
            return true;
        }
    }

    public int getAmountOfItems() {
        return amountProgressItems;
    }

    public boolean removeProgressItem(int byteAmount) {
        if (amountProgressItems > 0) {
            totalAmount = totalAmount - byteAmount;
            amountProgressItems--;
        }
        return updateProgressbarPercentage();
    }

    private int calculateProgressPercentage() {
        double x = progressAmount;
        double y = totalAmount;
        double result = (x / y) * 100;
        return (int) result;
    }

    private int round(int num) {
        int temp = num % 5;
        if (temp < 3)
            return num - temp;
        else
            return num + 5 - temp;
    }

    public void clear() {
        totalAmount = 0;
        progressAmount = 0;
        amountProgressItems = 0;
        progressBarPercentage = 0;
    }

}

It's really hard to tell what this class does. From what you say, I feel like you have a number of TransferTasks and want to have a sort of ProgressTracker for them. So, as for me, your goal is to make user's code look like this:

ProgressTracker progressTracker = new ProgressTracker();
progressTracker.setListener(new ProgressTrackerListener() {
  @Override
  public void onProgressChanged(double progressPercentage) {
    // TODO: update your UI here
  }
});
...
TransferTask transferTask = new UploadTask("/home/jiduvah/1.txt");
progressTracker.track(transferTask);
transferTask.start();

Why? Because it doesn't need any comments to describe what happens here: you just write it in English.

Update - In case you're absolutely sure you like your approach.

First, in this code:

private int round(int num) {
    int temp = num % 5;
    if (temp < 3)
        return num - temp;
    else
        return num + 5 - temp;
}

I see magic like 5 and 3. Looks like it does exactly what you need to do, and you've even wrote about meaning of 5, but 3 is still a mystery.

Then:

double x = progressAmount;
double y = totalAmount;
double result = (x / y) * 100;
return (int) result;

When you divide int by int, you get int. When you divide int by double or double by int, you get double. So you may just write:

double result = 100 * progressAmount / (double)totalAmount;

Regarding the whole idea, I'm not sure whether this code works at all, because it's hard to understand how one should use it.


Binding a keyboard key to a Tkinter button I'm specifically asking about binding a keyboard keystroke to a button in a GUI. Originally I planned to bind the keystroke to the GUI button itself, but that seemed impossible, and potentially unwise after I considered it later.

Is this the proper (pythonic) way to bind a keyboard key and GUI button?

## start.py
print 'start'

#from tkinter import *
from Tkinter import *
import os
from PIL import ImageTk, Image

class MyClass:
    '''This is my class.'''

    imgpath = '/Users/user/Dropbox/Camera Uploads'
    imgfile = 'susshi.jpg'
    #imgfilepath = os.path.join(self.imgpath, self.imgfile)
    imgfilepath = os.path.join(imgpath, imgfile)

    def __init__(self,master):
        print 'in __init__'

        # Create and load a frame into the tk (tkinter) window.
        frame = Frame(master)
        frame.pack()

        # Create and load two buttons in to the above-created frame.
        self.button = Button(frame, text="QUIT", fg="red", command=quit)
        self.button.pack(side=LEFT)
        self.slogan = Button(frame, text="Hello", command = self.f)
        self.slogan.pack(side=RIGHT)

        # Bind a key to each button from above.
        master.bind('q', quit)
        master.bind('f', self.f)

        # Open the image, resize it...
        self.image = Image.open(self.imgfilepath)
        self.image.thumbnail((600,600))
        # Make a tkinter-friendly image object ...
        self.display_image = ImageTk.PhotoImage(self.image)
        # Make a tkinter canvas, plug in the image, and pack the canvas.
        self.canvas = Canvas(master, bg='red')
        self.canvas.create_image(0,0, image=self.display_image, anchor="nw")
        self.canvas.pack(fill=BOTH, expand=1)
        ### FIXME: Set canvas focus!

    def f(self, event=None):
        print 'in f()'
        return 'hello world'

    def get_image_list(self):
        pass

    def next_image(self):
        pass

    def previous_image(self):
        pass

    def load_image(self):
        #stub
        pass

    def update_image(self):
        pass

root = Tk()
abc = MyClass(root)
root.mainloop()

print 'abc has been defined'
print '"abc.f()" returns "%s"' % abc.f()
#print str(abc.f)
#print repr(abc.f)

print 'end'

The intention of the project is to make a simple image viewer in python, with buttons in the GUI for basic navigation functions, and keys mapped to those same functions.

TL;DR: Yes, binding the keyboard key and button to the same function is a proper solution.

How to bind a keyboard key to a tkinter button

There are two ways to share a function between a bind keypress a command button.

Method 1: Use an optional event parameter

As you have already discovered yourself, you can use an optional event parameter that you don't depend on.

Since the parameter is unused, you can prefix it with an underscore (_event) to prevent warnings.

    self.button = Button(frame, text="Hello", command=self.func)
    master.bind('f', self.func)

def func(self, _event=None):
    print("Hello, world")

Full Example:

# from tkinter import Button, Frame, Tk  # Python 3
from Tkinter import Button, Frame, Tk    # Python 2

class MyClass:
    def __init__(self, master):
       frame = Frame(master)
       frame.pack()

       self.button = Button(frame, text="Hello", command=self.func)
       self.button.pack(side='left')

       master.bind('f', self.func)

    def func(self, _event=None):
        print("Hello, world")

root = Tk()
abc = MyClass(root)
root.mainloop()

Method 2: Use lambdas

If the event parameter isn't being used, you can discard it by binding your key to a lambda function.

    self.button = Button(frame, text="Hello", command=self.func)
    master.bind('f', lambda event: self.func())

def func(self):
    print("Hello, world")

Full Example:

# from tkinter import Button, Frame, Tk  # Python 3
from Tkinter import Button, Frame, Tk    # Python 2

class MyClass:
    def __init__(self, master):
        frame = Frame(master)
        frame.pack()

        self.button = Button(frame, text="Hello", command=self.func)
        self.button.pack(side='left')

        master.bind('f', lambda event: self.func())

    def func(self):
        print("Hello, world!")

root = Tk()
abc = MyClass(root)
root.mainloop()

(See How do I make Bind and Command do the same thing in tkinter?)

Miscellaneous tips

Avoid using wildcard imports

You should try to be explicit whenever you import in Python.

Yes

from Tkinter import Tk, Button, Frame

Yes

import Tkinter as tk

No

from Tkinter import *

(See Why is “import *” bad?)

Switch to Python 3

As coal_'s answer mentioned, development on Python 2 is scheduled to end in 2020. To ensure continued support, you should transition to Python 3 or later.

Use strings when packing constants

This is more personal preference than a rule, but I usually try to use strings when packing in tkinter. It means you don't have to import as many things.

Yes

self.button.pack(side="left")

No

self.button.pack(side=LEFT)


Take N elements from List of Lists I've come with a solution super tricky for a simple requirement. I think I could solve the problem using LinQ, but I'm not seeing it so clearly at all.
What's sure, I'm not comfortable with my code.

The requirement is the following:

Given a list of lists of string (IEnumerable>), take the first N elements from each but one by one.
So given this list of list and N=10:

{aaa, bb, ccc, ddd}
{eee, fff}
{ggg, hhhhh, iii, jjj, kkk}
{lll}
{1111, 22, 333, 444, 55555, 66666}

This is the output:

{aaa, eee, ggg, lll, 111, bb, fff, hhhhh, 22, ccc}

And here is the code:

private IEnumerable<string> Extract(
    IEnumerable<IEnumerable<string>> listOfList,
    int N
)
{
    var result = new List<string>();

    for (int i = 0; i < N; i++)
    {
        foreach (IEnumerable<string> list in listOfList)
        {
            if (list.Count() > i)
            {
                result.Add(list.ElementAt(i));

                if (result.Count >= N)
                    return result;
            }
        }
    }

    return result;
}

The code works, but I don't think this is maintainable nor easy to read.
Thanks.

Calling ElementAt() inside a loop will cause the IEnumerable to be called multiple times.  This code isn't deferring execution either.

Now if you think the previous code wasn't readable then hold on to your hats.  I'm going to post some code in hopes someone can build on it and maybe come up with something better.

public static class IEnumerableExtensions
{
    public static IEnumerable<TSource> Interweave<TSource>(this IEnumerable<TSource> source, params IEnumerable<TSource>[] weavers)
    {
        // Create a list of Enumerators but need to reverse it as will be removing from list and don't want o mess up indexer
        var enumerators = new[] { source }.Concat(weavers).Select(x => x.GetEnumerator()).Reverse().ToList();
        try
        {
            while (enumerators.Count > 0)
            {
                //  index backwards so we can remove from list and not mess up index
                for (var i = enumerators.Count - 1; i >= 0; i--)
                {
                    var currentEnumerator = enumerators[i];
                    if (currentEnumerator.MoveNext())
                    {
                        yield return currentEnumerator.Current;
                    }
                    else
                    {
                        currentEnumerator.Dispose();
                        enumerators.Remove(currentEnumerator);
                    }
                }
            }
        }
        finally
        {
            // finally block as we can't use "using" as have multiple disposables and don't know count ahead of time
            if (enumerators != null)
            {
                enumerators.ForEach(x => x.Dispose());
            }
        }
    }
}

This doesn't do Take, but you can just chain on the Take method.

Example of it in use:

static void Main(string[] args)
{
    var one = new[] { "aaa", "bb", "ccc", "ddd" };
    var two = new[] { "eee", "fff" };
    var threee = new[] { "ggg", "hhhhh", "iii", "jjj", "kkk" };
    var four = new[] { "lll" };
    var five = new[] { "1111", "22", "333", "444", "55555", "66666" };

    foreach (var item in one.Interweave(two, threee, four, five).Take(6))
    {
        Console.WriteLine(item);
    }
    Console.ReadLine();
}


Running 2 sets of tasks at the same time Currently, my team and I have 2 types of databases that are updated periodically, SQL Server and Access. Also, we run specialized reports based on those databases.

Basically, we have 4 tasks that need to be completed:

Update SQL Server
Publish SQL Server Reports
Update Access Databases
Publish Access Db reports

Access takes forever to run, so I am trying to make my code run the task of Updating Access and Publishing Access reports at the same time that I am updating SQL Server and publishing those reports.

Can someone verify if I am on the right path, or maybe share a link that better explains async?

static void Main(string[] args)
{
    var cs1 = new _1_UpdateSQL();
    var cs2 = new _2_PublishSQLRpts();

    Task t = TriggerAccess();

    cs1.UpdateSqlCodeOnly();
    cs2.PublishSqlRptsCodeOnly();
    t.Wait();
}

static async Task TriggerAccess()
{
    var cs3 = new _3_UpdateAccessDb();
    var cs4 = new _4_PublishAccessRpts();

    await Task.Run(() => cs3.UpdateAccessDbCodeOnly());
    await Task.Run(() => cs4.PublishAccessRptsCodeOnly());

}

When I have to manage multiple threads, I make sure all processing is complete, typically by using WaitAll.

List<Task> tasks = new List<Task>();
tasks.Add(Task.Run(() => { TriggerAccess(); }));
tasks.Add(Task.Run(() => { TriggerSQL(); }));
Task.WaitAll(tasks.ToArray());

This also allows me to use methods in a non-threaded capacity for testing and to easily add additional threads when the need arises. I can then also return actual data from a method, if I need to, instead of just a thread.

The difference between running the method in a thread and having the method be an async thread is that an async method waits until the first await statement to return to the main thread for processing whereas threading the method immediately continues to the next statement in the main method.

http://blogs.msdn.com/b/pfxteam/archive/2011/10/24/10229468.aspx and http://msdn.microsoft.com/en-us/library/hh156513.aspx are very helpful for me.


Assembling a Sequence I've completed the "Sequence Counter" level of TIS-100, but this is horrendously inefficient.  My cycle counts are at about twice the minimum possible according to the charts:

I'm not really as concerned about the other metrics.

I struggled to figure this one out.  And there's definitely bits of my code which I know look suspicious, but I don't feel strong enough with the language yet to really spot how to make this execute fewer instructions.

Here's the problem description:

SEQUENCES ARE ZERO TERMINATED
READ A SEQUENCE FROM IN
WRITE THE SUM TO OUT.S
WRITE THE LENGTH TO OUT.L

Most of my code are in three nodes:

TOP LEFT (IN)

START:
 MOV UP, ACC
 MOV ACC, DOWN
 MOV ACC, DOWN

JNZ SEQ
 MOV 0, RIGHT
 MOV 0, RIGHT
 JMP START

SEQ:
 MOV 1, RIGHT
 MOV 1, RIGHT

TOP RIGHT

START:
 ADD LEFT
 SAV
 MOV 0, ACC
 MOV LEFT, ACC
 JEZ DONE
 SWP
 JMP START

DONE:
 SWP
 MOV ACC, DOWN
 MOV 0, ACC

MIDDLE LEFT:

START:
 ADD UP
 SAV
 MOV 0, ACC
 MOV UP, ACC
 JEZ DONE
 SWP
 JMP START

DONE:
 SWP
 MOV ACC, DOWN
 MOV 0, ACC

MIDDLE RIGHT:

MOV UP, DOWN

BOTTOM LEFT (OUT.S)

MOV UP, DOWN

BOTTOM RIGHT (OUT.L)

MOV UP, DOWN

As I said, this works, but it's horribly inefficient.  Cycle count is up to 485... but looks like it could be under 250.

Also, any tips on making this code more readable while still staying within the extreme character limits would be greatly appreciated.

Can't give you any pointers towards readibility; seems like you're doing all you can for that within the character/space limits.

As for the code: I see you reset the ACC twice. From Middle Left:

MOV 0, ACC
MOV UP, ACC

And again in the top right one. This is wasting precious cycles. You don't have to set ACC to 0 if you're writing in from a port in the next instruction. If I disable those, I get your program down to 407 cycles.

But by far the greatest improvement can be made by leveraging the TIS-100's parallel processing power! Try moving the addidion logic down to the bottom row, make the top-most block send its accumulator down only once and use the middle block to double the signal. This kills 1 cycle on the input pipe and this adds up to 30 cycles over all the input values combined. Doing ths as well with the 1/0 you send down the right path will save 30 c again. I got you down to 264 cycles this way!

Have fun trying this out.


Check all permutations of generic type parameters I have a function that takes an object ("token") and three additional parameters of different types ("obj1", "obj2", "obj3").  The object "token" has an 'IsSatisfied' method that may take none, 1, 2, or 3 paremeters of types matching "obj1", "obj2", "obj3".  Versions of "token" that have 1 or more parameters are generic.  Right now, I have an inelegant solution of pattern matching the "token" against every permutation of 0, 1, 2, or 3 parameters, as shown in the code sample below.

Is there a more elegant way to write this?  It works as is, but I might go to 4 parameters ("obj1", "obj2", "obj3", "obj4") in which case the list of permutations gets really long. Perhaps a solution that uses 'params' for an indefinite number of parameters.  I would also be happy to drop the generic parameters  on the Tokenizer class.

public class Tokenizer <T1, T2, T3>
{

    public void Test(IBoolToken token, T1 obj1, T2 obj2, T3 obj3)
    {
        if (token is IBooleanTokenable tokenable0P) tokenable0P.IsSatisfied();
        else if (token is IBooleanTokenable<T1> tokenable1P1) tokenable1P1.IsSatisfied(obj1);
        else if (token is IBooleanTokenable<T2> tokenable1P2) tokenable1P2.IsSatisfied(obj2);
        else if (token is IBooleanTokenable<T3> tokenable1P3) tokenable1P3.IsSatisfied(obj3);
        else if (token is IBooleanTokenable<T1, T2> tokenable2P1) tokenable2P1.IsSatisfied(obj1, obj2);
        else if (token is IBooleanTokenable<T1, T3> tokenable2P2) tokenable2P2.IsSatisfied(obj1, obj3);
        else if (token is IBooleanTokenable<T2, T3> tokenable2P3) tokenable2P3.IsSatisfied(obj2, obj3);
        else if (token is IBooleanTokenable<T2, T1> tokenable2P4) tokenable2P4.IsSatisfied(obj2, obj1);
        else if (token is IBooleanTokenable<T3, T1> tokenable2P5) tokenable2P5.IsSatisfied(obj3, obj1);
        else if (token is IBooleanTokenable<T3, T2> tokenable2P6) tokenable2P6.IsSatisfied(obj3, obj2);
        else if (token is IBooleanTokenable<T1, T2, T3> tokenable3P1) tokenable3P1.IsSatisfied(obj1, obj2, obj3);
        else if (token is IBooleanTokenable<T1, T3, T2> tokenable3P2) tokenable3P2.IsSatisfied(obj1, obj3, obj2);
        else if (token is IBooleanTokenable<T2, T1, T3> tokenable3P3) tokenable3P3.IsSatisfied(obj2, obj1, obj3);
        else if (token is IBooleanTokenable<T2, T3, T1> tokenable3P4) tokenable3P4.IsSatisfied(obj2, obj3, obj1);
        else if (token is IBooleanTokenable<T3, T1, T2> tokenable3P5) tokenable3P5.IsSatisfied(obj3, obj1, obj2);
        else if (token is IBooleanTokenable<T3, T2, T1> tokenable3P6) tokenable3P6.IsSatisfied(obj3, obj2, obj1);
    }

}
```

If this is a comment theme throughout your code I would say to make an IArgument interface that would manage your object parameters. But if this is the only time you are dealing with these obj parameters, I would say pass the arguments of IsSatisfied in as generic objects and let IsSatisfied worry about ordering the arguments by type:

    void IsSatisfied(params object[] objects)
    {
        // T1 obj1 = <find T1 in objects>;
        // T2 obj2 = <find T2 in objects>;
        // T3 obj3 = <find T3 in objects>;
        // continue with previous implementation of IsSatisfied
    }

You would need to deal with the case when more than one of the objs is the same type.

Overall I don't like how the Test() method smells. You're passing in obj1, obj2, obj3, and possibly even obj4, every time, but you may only use obj2. What are obj1 and obj3 in this case?


split string into container on char; efficiently? Here is my single parse code for splitting a std::string on a char into a std::vector<std::string>:

#include <iostream>
#include <vector>

std::vector<std::string> split_uri(std::string, unsigned chr='/');

int main() {
    std::vector<std::string> uri_v = split_uri(uri, '/');

    for (auto elem: uri_v)
         std::cout << elem << std::endl;
}

std::vector<std::string> split_uri(std::string uri, unsigned chr) {
    bool start=true;
    std::string part;
    std::vector<std::string> vec;
    vec.reserve(uri.length()/2);

    for(char c: uri) {
        switch(c) {
            case '0':
            case '1':
            case '2':
            case '3':
            case '4':
            case '5':
            case '6':
            case '7':
            case '8':
            case '9':
            case 'a':
            case 'b':
            case 'c':
            case 'd':
            case 'e':
            case 'f':
            case 'g':
            case 'h':
            case 'i':
            case 'j':
            case 'k':
            case 'l':
            case 'm':
            case 'n':
            case 'o':
            case 'p':
            case 'q':
            case 'r':
            case 's':
            case 't':
            case 'u':
            case 'v':
            case 'w':
            case 'x':
            case 'y':
            case 'z':
                part += c;
                break;
            default:
                if (c == chr) {
                    if (!start)
                        vec.push_back(part);
                    else start = false;
                    part.clear();
                    part += c;
                }
                else {
                    std::cerr << "Invalid URI; from: \'" << c << "\' (hex: " << std::hex << "0x" << (unsigned int)c << ")\n";
                    vec.clear();
                    return vec;
                }
        }
    }

    return vec;
}

What can I do to improve its efficiency?

First, consider passing potentially expensive-to-copy objects such as std::string as const reference rather than by-value. The obvious approach is to use std::istringstream and std::getline.

std::vector<std::string> split_uri(const std::string uri&, char chr)
{
    std::istringstream iss(uri);
    std::vector<std::string> vec;

    for(std::string token; getline(iss, token, chr); )
        vec.push_back(token);

    return vec;
}

With the input being \bar\can, the output is {"", "bar", "can"}.

However, std::istringstream is not known for its speed. If you don't want to use std::istringstream, you don't have to. The following should be much faster:

std::vector<std::string> split_uri(const std::string& uri, char chr)
{
    std::string::const_iterator first = uri.cbegin();
    std::string::const_iterator second = std::find(first+1, uri.cend(), chr);
    std::vector<std::string> vec;

    while(second != uri.cend())
    {
        vec.emplace_back(first, second);
        first = second;
        second = std::find(second+1, uri.cend(), chr);
    }

    vec.emplace_back(first, uri.cend());

    return vec;
}

With the input being \bar\can, the output is {"\bar", "\can"}.


getting inferences working with typescript pubsubber So I'm learning typescript and I'm making a simple functional PubSub system. I want the user to be able to define topics and the data for those topics. So far I've got the below code, but I really want to improve it.
I'm not sure if I can get rid of the unknown in <T extends Record<string,unknown>>. I don't think that's a huge issue for me. I'd really like to get rid of the Set<Subscriber> part of the map that holds the subscribers. I tried something simple like Set<Subscriber<T[keyof T]>> but then it complains below where I am using <R extends keyof T>. Maybe that's the part that needs fixing?
Any and all suggestions welcome of course!
type Subscriber<R> = (data: R) => void;

const pubSubber = <T extends Record<string, unknown>>() => {
  const subscribers = new Map<keyof T, Set<Subscriber<any>>>();
  return {
    subscribe: <R extends keyof T>(topic: R, subscriber: Subscriber<T[R]>) => {
      const current = (subscribers.get(topic) || new Set()) as Set<
        Subscriber<T[R]>
      >;
      current.add(subscriber);
      subscribers.set(topic, current);
      return () => current.delete(subscriber);
    },
    publish: <R extends keyof T>(topic: R, data: T[R]) => {
      const toCall = Array.from((subscribers.get(topic) || [])) as Subscriber<T[R]>[];
      toCall.map((f) => f(data));
    },
  };
};

type Topics = {
  gear_with_radius: {
    center: number;
    radius: number;
  };
  outside: {
    help: boolean;
  };
};
const myPubSubber = pubSubber<Topics>();
myPubSubber.publish('gear_with_radius', { center: 10, radius: 5 });

I have a few pointers regarding your snippet.
Return type
Let's start with pubSubber function return type. It's very nice that typescript infers types, but it also a good way to make a mistake. The bad thing about it is that you can accidentally change the return type without knowing it. If you use this function somewhere in the app there is a chance that typescript will notify you about the change. But if you are creating a library and don't have tests, you will change the return type without any error.
Personally, I like the rule to always specify return type (unless using inline arrow functions). I even use the lint rule to enforce this.
Don't use any
Change any to unknown. In this case, maybe it doesn't matter at least at the moment, but in the future, you may change this code. With any you can do anything and typescript won't say anything. With unknown you can't do anything, unless you cast it or use type Type Guard
const subscribers = new Map<keyof T, Set<Subscriber<any>>>()

Don't use cast if possible
Let's look into this line.
const current = (subscribers.get(topic) || new Set()) as Set<
  Subscriber<T[R]>
>;

current real type is Set<Subscriber<any>>, but you cast it to Subscriber<T[R]>. But in this example, there is no point having cast at all, because even without it the code is correctly typed. This function implementation works with any type, so you can leave it as it is. As for the outer interface, it can be more strict.
const current = subscribers.get(topic) || new Set()
current.add(subscriber);
subscribers.set(topic, current);
return () => current.delete(subscriber);

Second example is a bit better:
const toCall = Array.from((subscribers.get(topic) || [])) as Subscriber<T[R]>[];
toCall.map((f) => f(data))

You already had one mistake because of this cast. Also, I don't see any reason to convert Set to the list. You can just loop the Set. And if you combine it with optional chaining, it will look like:
subscribers.get(topic)?.forEach(f => f(data));

Better typing
In the previous section, I mentioned casts. But removing casts in theory you lose some typings (adding also lose some typings, dilemma...). Let's try to have full correct typings without any cast. So to start with we need to change Map to Object. Map types do not allow relations between key and value (e. g.: a key must-have number value`).
Example of relation-based subscribers list:
const subscribers: {
  [P in keyof T]?: Set<Subscriber<T[P]>>
} = {}

Final solution
So the final solution can looks like:
type Subscriber<R> = (data: R) => void;
type BaseTopics = Record<string, unknown>

interface PubSubber<T extends BaseTopics> {
  subscribe: <R extends keyof T>(topic: R, subscriber: Subscriber<T[R]>) => () => void
  publish: <R extends keyof T>(topic: R, data: T[R]) => void
}

function pubSubber<T extends BaseTopics>(): PubSubber<T> {
  const subscribers: {
    [P in keyof T]?: Set<Subscriber<T[P]>>
  } = {}
  return {
    subscribe(topic, subscriber) {
      const current = subscribers[topic] || new Set();
      current.add(subscriber);
      subscribers[topic] = current;
      return () => current.delete(subscriber);
    },
    publish(topic, data) {
      subscribers[topic]?.forEach(f => f(data));
    },
  };
};