Using "Generics" to implement "Unit Of Work" I need to design a generic "Unit Of Work", basically so that I can "change the context of the Database" only changing the implementation of IUnitOfWork on my DI container. I will exemplify below.
So far, I have following solution:
IUnitOfWork.cs
public interface IUnitOfWork
{
    TTransactionType GetCurrentTransaction<TTransactionType>();

    Task CommitAsync(CancellationToken cancellationToken);
    Task BeginTransactionAsync(CancellationToken cancellationToken);
}

UnitOfWorkMongoDB.cs
public class UnitOfWorkMongoDB : IUnitOfWork
{
    IClientSessionHandle _session;
    readonly IMongoDbContext _mongoDbContext;

    public UOLMongoDB(IMongoDbContext mongoDbContext)
    {
        _mongoDbContext = mongoDbContext;
    }

    public TTransactionType GetCurrentTransaction<TTransactionType>() => (TTransactionType)_session;

    public async Task CommitAsync(CancellationToken cancellationToken)
    {
        await _session.CommitTransactionAsync();
    }

    public async Task BeginTransactionAsync(CancellationToken cancellationToken)
    {
        _session = await _mongoDbContext.MongoClient.StartSessionAsync(cancellationToken: cancellationToken);
        _session.StartTransaction();
    }
}

DI Resolution
 services.AddScoped<IUnitOfWork, UnitOfWorkMongoDB>();

Using the solution
public class MyServiceA
{
    public MyServiceA(IUnitOfWork unitOfWork)
    {
        /*...*/
    }
}

public class MyServiceB
{
    public MyService(IUnitOfWork unitOfWork)
    {
        /*...*/
    }
}

Considerations

The IUnitOfWork interface is not really generic, the only
"generic thing" used, was TTransactionType, since each Database
driver manufacturer has a different implementation of transaction
handling. Like:

MongoDB = IClientSessionHandler.cs
MySql = MySqlTransaction.cs
Oracle = OracleTransaction.cs

The IUnitOfWork interface is non generic in attempt to avoid refactoring
code when we change his implementation on D.I resolution.

Samples with non generic IUnitOfWork:
 services.AddScoped<IUnitOfWork, UnitOfWorkPostgreSQL>(); //OR
 services.AddScoped<IUnitOfWork, UnitOfWorkOracle>(); //OR
 services.AddScoped<IUnitOfWork, UnitOfWorkMySQL>(); //OR

Dependencies not break:
public class MyServiceA
{
    public MyServiceA(IUnitOfWork unitOfWork)
    {
        /*...*/
    }
}

"Problems" with with a generic IUnitOfWork<TUnitOfWork>.

If we change the resolution on D.I, all services dependecies will break, like:
services.AddScoped<IUnitOfWork<UnitOfWorkPostgreSQL>, UnitOfWorkPostgreSQL>();

On MyServiceB.cs for example, before he depends of IUnitOfWork<UnitOfWorkMongoDB> and now, my D.I container not solves it. This can cause several changes on application code base.
public class MyServiceB
{
    public MyServiceB(IUnitOfWork<UnitOfWorkMongoDB> unitOfWork)
    {
        /*...*/
    }
}

I still feel uncomfortable with the explicit conversion (TTransactionType)_session; on GetCurrentTransaction<TTransactionType>() method, is there any other approach to make this method? Or am I worrying too much?
What do you think can be improved in this solution? Any evaluation/improvement is welcome!
P.S I created a little repo with this implementation here.

Trying to not promote low-level transaction implementation in initial design,
IDbContext is the place for db-transactions.
 public interface IDbContext
 {
      void Commit();
      void Rollback();
 }

Using “Generics” to implement “Unit Of Work”

public interface IUnitOfWork<T> where T: IDbContext
{
   T DbContext { get; }
}

Now the async version of IUnitOfWork
public interface IAsyncUnitOfWork: IUnitOfWork<IDbContext>
{
    Task CommitAsync(CancellationToken cancellationToken);
    Task BeginTransactionAsync(CancellationToken cancellationToken);
}

Trying to implement abstract system with our setup.
public abstract class AbstractUnitOfWork: IAsyncUnitOfWork
{
  public abstract IDbContext DbContext {get; set; }

  public AbstractUnitOfWork(IDbContext context)
  {
    DbContext = context;
  }

  public abstract Task BeginTransactionAsync(CancellationToken cancellationToken);
  public abstract Task CommitAsync(CancellationToken cancellationToken);
}


Weighting mysql full-text search results by date I'm using innodb full-text search to return a set of results where the newest posts are more relevant.  Just wanted to check that this the best method:

SELECT *, (MATCH(note) AGAINST ('$search' IN BOOLEAN MODE))/SQRT(GREATEST(1, DATEDIFF(NOW(), date))) AS score FROM notes WHERE userid = '$userid' HAVING score > 0 ORDER BY score DESC

A few "small" comments.

Don't use SELECT * for reasons outlined in this question's answers:

What is the reason to not use select *?
Linebreaks and indentation make your code much easier to read:

SELECT *, (MATCH(note) AGAINST (
    '$search' IN BOOLEAN MODE))/SQRT(GREATEST(1, DATEDIFF(NOW(), date)
    )) AS score
FROM notes
WHERE userid = '$userid'
HAVING score > 0
ORDER BY score DESC

Things I like:

You're not writing it ALL IN SHOUT-CASE ;)


Rails validating enum The Ruby on Rails framework doesn't allow you to validate enums submitted through a form in any sane manner. Evidently, enums are meant to be used to maintain internal application state, and are not supposed to be user-facing. Well, in my case it doesn't matter too much, but I still want validations *just in case*.

Here is a widely-used enum:

class Language < ActiveRecord::Base
    self.abstract_class = true

    enum language: {
        arabic: 0,
        chinese: 1,
        english: 2,
        french: 3,
        german: 4,
        italian: 5,
        japanese: 6,
        russian: 7,
        spanish: 8,
        other: 9
    }
end

In my schema, the field using this enum is a text array: t.text     "audio_languages",    default: [], allowing multiple languages to be selected through the form.

In the form, the values are added as a list of check boxes:

In the controller, the values for that field come in as:

"audio_languages"=>["English", ""]

Notice the last array entry, "". That's just how it works I guess, nothing I've tried gets rid of that, so I have to handle it in the validation.

In the validation, a language needs to be chosen, and the chosen language has to be present in the language enum:

def validate_audio_languages

    # Remove the empty string, which is an artifact of form submission of an array
    # field, during validation
    audio_languages.reject! { |l| l.empty? }

    if !audio_languages.any?
        errors.add(:audio_languages, "need to select a language")
    end

    # If any of the audio languages is invalid, add the error and break
    audio_languages.each do |al|
        valid = false
        Language.languages.each do |l, i|
            valid = true if (al.downcase == l.downcase)
        end

        if !valid
            errors.add(:audio_languages, "not a valid language selection")
            break
        end
    end
end

I'm concerned about the length of the function more than anything. Surely there's an easier way to do this kind of simple validation, especially to validate whether or not a value is actually in the enum. Any help in dealing with this issue is much appreciated!

You can use Ruby's array intersection and check for matches. The empty string passed in your params is irrelevant.

def validate_audio_languages
    if audio_languages.any?
        enums = Language.languages.map(&:first)
        matches = enums & audio_languages.map!(&:downcase)
        matches.present? ? matches : errors.add(:audio_languages, "not a valid language selection")
    else
        errors.add(:audio_languages, "need to select a language")
    end
 end

You can shave off a couple more lines at the expense of readability, but it pays to keep things simple and readable.


Java OOP Temperature Converter I am doing a college exercise and would like to know how to improve the execution of the idea. The idea is to create a program that converts temperature measurements.

Main.java

package app.cnv;

public class Main {
  public static void main(String[] args) {
    Celsius celsius = new Celsius(37);

    System.out.println(celsius.convert(new Fahrenheit()));

    Fahrenheit fahrenheit = new Fahrenheit(98.6);

    try {
      System.out.println(fahrenheit.convert(new Celsius()));
    } catch (AbsoluteZeroException e) {
      System.out.println(e.getMessage());
    }
  }
}

Temperature.java

package app.cnv;

public abstract class Temperature {
  public abstract double convert(Temperature to) throws AbsoluteZeroException;
}

Celsius.java

package app.cnv;

public class Celsius extends Temperature {
  double celsius;

  Celsius() {
  }

  Celsius(double celsius) {
    this.celsius = celsius;
  }

  @Override
  public double convert(Temperature to) {
    if (to instanceof Fahrenheit) {
      return (celsius * 9) / 5 + 32;
    }
    return celsius;
  }
}

Fahrenheit.java

package app.cnv;

public class Fahrenheit extends Temperature {
  double fahrenheit;

  Fahrenheit() {
  }

  Fahrenheit(double fahrenheit) {
    this.fahrenheit = fahrenheit;
  }

  @Override
  public double convert(Temperature to) throws AbsoluteZeroException {
    if (to instanceof Celsius) {
      if (fahrenheit < -459.67) {
        throw new AbsoluteZeroException("The temperature cannot be less than absolute zero.");
      } else {
        return ((fahrenheit - 32) * 5) / 9;
      }
    }
    return fahrenheit;
  }
}

AbsoluteZeroException.java

package app.cnv;

public class AbsoluteZeroException extends Exception {
  public AbsoluteZeroException(String err) {
    super(err);
  }
}

What if I add a class Kelvin without modifying the other classes? When I convert Celsius to Kelvin then, your code says that 20 °C = 20 K, which is simply wrong.

Your whole approach is doomed to fail. What you should do instead is to define one reference temperature scale. I would take Kelvin for that. Then, for every other temperature scale, have methods fromKelvin and toKelvin. That's all. This approach is well-known from constructing compilers, which has the same problem of several similar source languages and several different target machines.

For converting between arbitrary temperature scales you can have a helper method in the Temperature class like this:

public static Temperature convert(Temperature from, TemperatureScale to) {
    double kelvin = from.toKelvin();
    return to.fromKelvin(kelvin);
}

You can see that there are two completely different classes involved:

A temperature scale, like Kelvin or Fahrenheit
A specific temperature, consisting of a temperature scale and a value, like 293 Kelvin

Regarding the exception class: There is no good reason to give it a String parameter. The name of the class is already expressive enough. Therefore you should remove that parameter and just throw new AbsoluteZeroException(). You should probably rename it to BelowAbsoluteZeroException. Or you should just take the predefined IllegalArgumentException and be fine. In many cases you don't need to invent your own exception classes.


Fetching an array value and removing it So I have this array:

$arr = ('a' => 343, 'b' => 34, 'c' => 65, 'd' => 465);

which could also be

$arr = ('a' => 343, 'b' => 34, 'c' => 65);

Can the following code be improved into one line?

if(isset($arr['d'])) $something = $arr['d'];
unset($arr['d']);

if(isset($arr['d'])) $something = $arr['d'];
unset($arr['d']);

Well... If arr['d'] is not set, then you don't need to unset it. So this will save an unnecessary call sometimes:

if(isset($arr['d']))
{
    $something = $arr['d'];
    unset($arr['d']);
}

If you don't care for $something, then you can just do

unset($arr['d']);

Which is then one line. You could also write

if (isset($arr['d']) && ($something = $arr['d']) !== null) unset($ar['d']);

That is one line as well, but will fail to do what you like if $something is null. And from the point of readability it's questionable whether this is an improvement.


Best syntax for executing an action between each loop iteration. The question of the cleanest way to write a loop that executes some action between each iteration has always interested me.

In a sense, what is the best way in c/c++ to implement this fictional construct:

for (...) {
} between {
}

Using such a construct you could trivially implement string join(): (pseudocode)

result = "";
foreach ( item : input ) {
    result += str(item);
} between {
    result += sep;
}

I have looked through some popular code libraries to see how these types of loops are implemented, and there are some common strategies:

move the "between" code into an "if (!first/last iteration)" inside the loop.

This is the go-to method when the index/iterator/result freely stores the notion of first/last iteration (such as checks for values of 0, .empty(), NULL etc).

transform the "loop body" into a function and call it from two places: before and during the loop, and change the loop to skip the first element. (minor code duplication)

This is the go-to method when the "loop body" is a single function call

Neither of these is a completely generalized solution, and while its only a few lines + a state variable, I'm trying to find an ideal solution.

Priorities:

No needless source code duplication (but willing to accept binary code duplication)
Efficiency
Clear semantics
Trivially usable (aka simple syntax, few library requirements)

The closest I've come to this in C or C++ is a slight modification to Knuth's loop-and-a-half:

template<class T>
void print(std::vector<T> const &x) {
  std::cout << '[';
  typename std::vector<T>::const_iterator
    begin = x.begin(),
    end = x.end();
  if (begin != end) {  // Duplicated condition to handle the empty case.
    while (true) {  // Here is the "loop and a half".
      std::cout << *begin;
      if (++begin == end) break;
      std::cout << ", ";
    }
  }
  std::cout << "]\n";
}

C++0x allows you to generalize:

template<class Iter, class Body, class Between>
void foreach(Iter begin, Iter end, Body body, Between between) {
  if (begin != end) {  // This duplication doesn't matter as it is
                       // wrapped up in a library function.
    while (true) {
      body(*begin);
      if (++begin == end) break;
      between();
    }
  }
}

template<class T>
void print(std::vector<T> const &x) {
  std::cout << '[';
  foreach(x.begin(), x.end(),
    [](T const& v) {
      std::cout << v;
      // Long code here is still readable.
    },
    []{
      std::cout << ", ";
      // Long code here is still readable.
    });
  std::cout << "]\n";
}

Lambda capture even allows you to modify local variables in the function calling foreach.


Test code for custom malloc I've been working on a custom malloc where the test code is the following.

#include <stdlib.h>
#include <stdio.h>
#include <sys/types.h>
#include <unistd.h>
#include "../tst.h"
#include "../brk.h"
#include <time.h>
/* returns an array of arrays of char*, all of which NULL */
char ***alloc_matrix(unsigned rows, unsigned columns) {
    char ***matrix = malloc(rows * sizeof(char **));
    unsigned row = 0;
    unsigned column = 0;
    if (!matrix) abort();

    for (row = 0; row < rows; row++) {
        matrix[row] = calloc(columns, sizeof(char *));
        if (!matrix[row]) abort();
        for (column = 0; column < columns; column++) {
            matrix[row][column] = NULL;
        }
    }
    return matrix;
}

/* deallocates an array of arrays of char*, calling free() on each */
void free_matrix(char ***matrix, unsigned rows, unsigned columns) {
    unsigned row = 0;
    unsigned column = 0;
    for (row = 0; row < rows; row++) {
        for (column = 0; column < columns; column++) {
            /*    printf("column %d row %d\n", column, row);*/
            free(matrix[row][column]);
        }
        free(matrix[row]);
    }
    free(matrix);
}

int main(int agrc, char **argv) {
     int i;
    srand(time(NULL));
    int randomnumber;
    int size = 1024;
    void *p[size];
    for (i = 0; i < size; i++) {
        randomnumber = rand() % 10;
        p[i] = malloc(1024 * 1024 * randomnumber);
    }

    for (i = size-1; i >= 0; i--) {
        free(p[i]);
    }

    int x = 1024;
    char *** matrix = alloc_matrix(x, x);
    free_matrix(matrix, x, x);
    return (0);
}

The code compiles and runs cleanly.

developer@1604:~/proj/openmalloc/overhead$ gcc gigtest.c
developer@1604:~/proj/openmalloc/overhead$ ./a.out

But how can I improve it? I would like to make it easier to test the thing (my next work should be to add multithreading to the malloc implementation).

Should I use a testing framework instead?

This code is very easy to understand, which is great! I like that you used full words for the variable names rather than abbreviating everything to single character names.
Don't Abort
In alloc_matrix(), if there is a failure to allocate memory, it calls abort(). This is going to make it very difficult to test! If it attempts to allocate and fails, it aborts the test. Usually a test will return success or failure. This is an important distinction because you may write tests that should fail to test the failure paths of your code.
Proper Types
I would change the types of the argument to this:
char*** alloc_matrix(const size_t rows, const size_t columns);

I made them all const because the function doesn't change their values. I also made them size_t which is the appropriate type to use when you want to represent a size.
More Tests
You should add some more tests. As mentioned above, you need to test cases that should fail, such as allocating a block that's the maximum 64-bit value (for example malloc(ULLONG_MAX);).
You should also test things like alignment. Many platforms have strict byte alignment requirements. For example, pointers can usually not have an odd start address, and often must be 4-byte, 8-byte, or 16-byte aligned. You may need to check the specifics of the platform you're testing on.


Is there a memory leak in here? I'm newish to obj-c and I have some code that works, but I'm thinking I may be leaking memory as I have two retain statements and no release statements.

In both cases if I remove the retain I get a bad access error on [btPairedDevices count]

Am I doing this right or is there a better way?

//===================================================================================================
- (id)init
{
    self = [super init];
    if (self) {
        indexDeviceSearch = 0;
        // Grab a list of paired devices
        btPairedDevices = [IOBluetoothDevice pairedDevices];
        [btPairedDevices retain];
    }

    return self;
}

//===================================================================================================
- (void) applicationDidFinishLaunching:(NSNotification *)notification
{
    if(IOBluetoothValidateHardwareWithDescription(nil, nil) != kIOReturnSuccess)
    {
        [NSApp terminate:self];
    }

    NSLog(@"Application up an running");

    // Start our timer looking for paired phones
    timerDeviceSearch = [NSTimer scheduledTimerWithTimeInterval:1.0
                                                         target:self
                                                       selector:@selector(timerDeviceSearchFired)
                                                       userInfo:nil
                                                        repeats:YES];
}

//===================================================================================================
- (void) timerDeviceSearchFired
{
    // Check out the device
    if (indexDeviceSearch < [btPairedDevices count])
    {
        IOBluetoothDevice *device = [btPairedDevices objectAtIndex:indexDeviceSearch];
        if ([device deviceClassMajor] == kBluetoothDeviceClassMajorPhone)
        {
            // Found a phone
            [self addNewDeviceIfAcceptable:device];
        }
    }

    // Update the list of devices just in case the use
    // paired something while we are running
    btPairedDevices = [IOBluetoothDevice pairedDevices];
    [btPairedDevices retain];

    // Inc the index
    indexDeviceSearch++;
    if (indexDeviceSearch >= [btPairedDevices count])
        indexDeviceSearch = 0;
}

//===========================================================================================================================
-(BOOL)addNewDeviceIfAcceptable:(IOBluetoothDevice*)device
{
    NSEnumerator *enumerator;
    IOBluetoothDevice *tmpDevice;
    const BluetoothDeviceAddress *newDeviceAddress = [device getAddress];

    // Allocate our array of monitored devices
    if(!btMonitoredDevices)
    {
        btMonitoredDevices = [[NSMutableArray alloc] initWithCapacity:1];
        if(!btMonitoredDevices)
            return(FALSE);
        [btMonitoredDevices retain];
    }

    // Walk the devices in the array.
    enumerator = [btMonitoredDevices objectEnumerator];
    if(enumerator)
    {
        const BluetoothDeviceAddress* tempAddress = NULL;
        while((tmpDevice = [enumerator nextObject]))
        {
            tempAddress = [tmpDevice getAddress];
            if(memcmp(newDeviceAddress, tempAddress, sizeof(BluetoothDeviceAddress)) == 0 )
            {
                // Already have it.
                return(FALSE);
            }
        }
    }

    // Add new device to array
    NSLog(@"Monitoring new device: %@", [device nameOrAddress]);
    [btMonitoredDevices addObject:device];

    // Return that we haven't seen it.
    return(TRUE);
}

You should definitely be releasing both btMonitoredDevices and btPairedDevices in the dealloc. But it appears that this is your appDelegate so it matters little outside of correctness. I'd do it anyhow.

You shouldn't need the retain on btMonitoredDevices since you're calling alloc on it. So you already have a retain count of one on it. By calling retain on it you actually have a retain count of two.

So to sum up make sure you're releasing both in your dealloc and get rid of the [btMonitedDecices retain]; and you should be good.


Stitching together badly formatted log files I needed to stitch together a large number of badly formatted log files. I [painfully] developed a script for it in R, one that successfully dealt with the various flaws in the dataset. However, this script is rather slow. Considering I'm a neophyte to the field (and don't know what I do not know), I was hoping I could get some suggestions on how to speed this code up - using either R, or any other Windows-based/CLI tools.

#Getting path for all files within a particular directory.
files = list.files(path = dirs[j], full.names = T)

#The files are in csv format and in the ideal case have exactly 5 columns. However, the 5th column can contain an arbitary number of commas. If I try to fread with sep = ",", certain rows can be of arbitarily high length. If I use select = 1:5 to subset each row, I lose data.

#My solution was to read each line into a single column and then seperate into columns within the script based on the location of the first 4 commas.
data <- rbindlist(lapply(files,fread,sep = "\n",fill = T,header = F))

#Removing empty rows.
retain <- unlist(lapply(data, function(x) {
       str_detect(x,".")
   }))
data[retain,] -> data

#Removing rows where there is no data in the 5th column.
retain <- unlist(lapply(data, function(x) {
       str_detect(trimws(x,which ='both') ,".+,.+,.*,.*,.+")
   }))
data[retain,] -> data

#This replaces the first 4 commas with a tab-delimiter.
for(i in 1:4){
data <- data.frame(lapply(data, function(x) {
                    str_replace(x,",","\t")
              }),stringsAsFactors = F)
}

#This splits the row into 5 seperate columns, always.
data <- unlist(lapply(data, function(x) {
  unlist(strsplit(x,"\t",fixed = T))
}))

#Changes the format from a character vector to a data table.
data = data.frame(matrix(data,ncol=5,byrow = T),stringsAsFactors = F)

Without knowing exactly your data structure, I have to guess that it looks like this:

data <- data.table(a = c('1,1,1,4,6',
                         '1,2,3,4,',
                         '',
                         '1,2,3,4,',
                         '1,2,3,4,5'))

If it is correct, then your desired operations could be done like this:

#############################################
#Removing empty rows.
data <- data[data[[1]] != '']

#Removing rows where there is no data in the 5th column.
retain <- str_detect(trimws(data[[1]], which = 'both') ,".+,.+,.*,.*,.+")
data <- data[retain, ]

#This replaces the first 4 commas with a tab-delimiter.
for(i in 1:4) data[[1]] <- str_replace(data[[1]], ",", "\t")

#This splits the row into 5 seperate columns, always.
dNew <- as.data.table(tstrsplit(data[[1]], "\t", fixed = T))
dNew
#    V1 V2 V3 V4 V5
# 1:  1  1  1  4  6
# 2:  1  2  3  4  5

The main problem is that you are using lapply where it is not needed.


Implementing a cross-browser AJAX utility as an exercise I am currently learning JavaScript and created this cross-browser AJAX utility as a learning exercise. It was not meant to be used in production code, and I just tried to cover only the cases that I'm aware of, so there might be a lot of edge cases that are not covered.

I'd be happy if you could check it and let me know how it looks and give me suggestions on how to improve it:

var ajax = {
    request: function(url, requestType, callback, queryString) {
        var ids = ['MSXML2.XMLHTTP.3.0',
                   'MSXML2.XMLHTTP',
                   'Microsoft.XMLHTTP'],
            xhr;

        if (typeof window.XMLHttpRequest === 'function') {
            xhr = new XMLHttpRequest();
        } else { // For IE versions below 7
            for (var i = 0; i < ids.length; i++) {
                try {
                    xhr = new ActiveXObject(ids[i]);
                    break;
                } catch(e) {}
            }
        }
        // Do I really need to put the callback in a
        // closure, as the xhr object gets recreated
        // each time ajax.request() is called?
        xhr.onreadystatechange = (function(myXhr) {
            return function() {
                callback(myXhr);
            };
        })(xhr);
        xhr.open(requestType, url, true);
        if (requestType.toUpperCase() === 'GET') {
            xhr.send('');
        } else if (requestType.toUpperCase() === 'POST') {
            xhr.send(queryString);
        }

    }
}

P.S.: This exercise is from the book "Object-Oriented JavaScript" by Stoyan Stefanov.

I can't specifically comment on your usage of the extra tries for < IE7, as I always make sure my audience is using something better.  It seems like there should be a more elegant way to check each of those than a try with an empty catch, but without knowing how IE treats them, I can't say what it would be.  The rest is okay, but I would suggest the following.  My changes are explained in my own comments.

var ajax = {
  request: function(url, requestType, callback, queryString) {
    var ids = ['MSXML2.XMLHTTP.3.0',
    'MSXML2.XMLHTTP',
    'Microsoft.XMLHTTP'],
    xhr;

    // Simplification of this check while essentially doing the same thing
    if (window.XMLHttpRequest) {
      xhr = new XMLHttpRequest();
    } else {
      for (var i = 0; i < ids.length; i++) {
        try {
          xhr = new ActiveXObject(ids[i]);
          break;
        } catch(e) {}
      }
    }
    // Calling a function to return a function is redundant.
    // Do what you're trying to with as little extra as possible.
    xhr.onreadystatechange = function() {
        callback(xhr);
      };
    xhr.open(requestType, url, true);
    if (requestType.toUpperCase() === 'GET') {
      // When initiating a get request, the send function needs no arguments at all.
      xhr.send();
    } else if (requestType.toUpperCase() === 'POST') {
      xhr.send(queryString);
    }
    // If you want to be extra careful, include an else here to handle a bad requestType
  }
}


How can I improve the isolation for this controller spec? So I'm doing things the BDD way, writing features first, then moving on to writing specs to add more detail to the mix as I move down the layers.

One thing that bothered me, but isn't a big problem, is that when I got to the controller part of the stack, I have calls to the User and Todo model in my controller spec.

Is this okay and normal? Or should I be stubbing these things out?

describe TodosController do
  let(:user) { Factory(:user) }

  describe "post 'create'" do
    lambda {
      post :create
    }.should change(user.todos, :count).by(1)
  end
end

It's fine to test how controllers alter models.


API controller with database queries I have the following controller with a couple of database queries and I'd like you to review it.

I've tried to make it clean for other developers but the communication with the database can be improved (more effective). The exception returned when nothing is found, so I'm not sure if it's the best way to do it.

public class LookupController : ApiController
{
    // GET api/Lookup
    [CacheOutput(ServerTimeSpan = 3600, ClientTimeSpan = 3600)]
    public List<GlassModel> Get(string filter)
    {
        if (string.IsNullOrEmpty(filter)) throw new ArgumentNullException(nameof(filter));

        using (var db = new LookupDbContext())
        {
            var salesCodes = GetSalesCodes(db, filter.ToUpper().Split(','));

            if (salesCodes.Any())
            {
                var glassQuantities = GetQuantitesByCountry(salesCodes, db);

                if (glassQuantities.Any())
                    return glassQuantities;
            }
            throw new SystemException("Value not found.");
        }
    }

    private List<GlassModel> GetQuantitesByCountry(List<string> salesCodes, LookupDbContext db)
    {
        var queryGlass = db.Countries
            .Where(c => salesCodes.Contains(c.Eurocode))
           .GroupBy(c => new { c.Countrycode })
           .Select(g => new
           {
               g.Key.Countrycode,
               Quantity = g.Sum(p => p.Quantity)
           });

        // build model
        return queryGlass.ToList().Select(x => new GlassModel
        {
            Coordinates = LocationHelper.GetCoordinatesByCountry(x.Countrycode),
            Country = x.Countrycode,
            Quantity = x.Quantity
        }).ToList();
    }

    private List<string> GetSalesCodes(LookupDbContext db, string[] filterValues)
    {
        var make = filterValues[0];
        IQueryable<Car> filter;

        if (filterValues.Length == 1) // make
        {
            filter = db.Cars.Where(c => c.VehicleMakeName.Contains(make));
        }
        else if (filterValues.Length == 2) // make, model
        {
            var carModel = filterValues[1].Trim();

            filter = db.Cars.Where(c => c.VehicleMakeName.Contains(make) &&
                                         c.VehicleModelName.Contains(carModel));
        }
        else // make, model, year
        {
            var model = filterValues[1].Trim();
            var year = Convert.ToDouble(filterValues[2]);

            filter = db.Cars.Where(c => c.VehicleMakeName.Contains(make) &&
                                          c.VehicleModelName.Contains(model) &&
                                          (c.FromYear <= year || c.FromYear == null) &&
                                          (c.ToYear >= year || c.ToYear == null));
        }

        return filter
            .GroupBy(c => new { c.SalesCode })
            .Select(g => g.Key.SalesCode).ToList();
    }
}

I see two improvements, one of which Gert Arnold pointed on in a comment.

If an API method returns a list, collection or IEnumerable of anything, worse case scenario you should return an empty collection. I wouldn't even recommend returning a 404 Not Found because you are searching for multiple things. If returning a single resource, a 404 Not Found is appropriate, but not for returning a collection.
Abstract away the searching and generation of your view models into their own service. This makes it much easier to test your controller.

Moving query logic into a service

This is a three part process:

Create an interface for your service that exposes the minimal methods and parameters required for your controller to work
Create the service class that implements the interface
Use this service object in your controller through its interface

First, the interface:

public interface IVehicleService
{
    List<GlassModel> GetGlassModelQuantitesByCountry(string[] filterValues);
}

This has a single method that accepts the filter values, and returns a list of view models. I called it IVehicleService because the actual entity your are querying against is a Car (which really is a "vehicle" in the generic sense).

Before diving into the implementation of the IVehicleService interface, let's look at how the controller will use it.

public class LookupController : ApiController
{
    // GET api/Lookup
    [CacheOutput(ServerTimeSpan = 3600, ClientTimeSpan = 3600)]
    public List<GlassModel> Get(string filter)
    {
        if (string.IsNullOrEmpty(filter)) throw new ArgumentNullException(nameof(filter));

        using (var db = new LookupDbContext())
        {
            IVehicleService service = new VehicleService(db);
            IEnumerable<string> filterValues = filter.ToUpper().Split(',');

            return service.GetGlassModelQuantitesByCountry(filterValues );
        }
    }
}

This makes the controller very slim and easy to read. The ugliness of the query is hidden away in the service class.

public class VehicleService : IVehicleService
{
    public VehicleService(LookupDbContext context)
    {
        this.context = context;
    }

    private LookupDbContext context;

    public List<GlassModel> GetGlassModelQuantitesByCountry(IEnumerable<string> filterValues)
    {
        var salesCodes = GetSalesCodes(filterValues);
        var queryGlass = context.Countries
            .Where(c => salesCodes.Contains(c.Eurocode))
            .GroupBy(c => new { c.Countrycode })
            .Select(g => new
            {
                g.Key.Countrycode,
                Quantity = g.Sum(p => p.Quantity)
            });

        // build model
        return queryGlass.ToList().Select(x => new GlassModel
        {
            Coordinates = LocationHelper.GetCoordinatesByCountry(x.Countrycode),
            Country = x.Countrycode,
            Quantity = x.Quantity
        }).ToList();
    }

    private GetSalesCodes(IEnumerable<string> filterValues)
    {
        var make = filterValues[0];
        IQueryable<Car> filter;

        if (filterValues.Length == 1) // make
        {
            filter = context.Cars.Where(c => c.VehicleMakeName.Contains(make));
        }
        else if (filterValues.Length == 2) // make, model
        {
            var carModel = filterValues[1].Trim();

            filter = context.Cars.Where(c => c.VehicleMakeName.Contains(make)
                                          && c.VehicleModelName.Contains(carModel));
        }
        else // make, model, year
        {
            var model = filterValues[1].Trim();
            var year = Convert.ToDouble(filterValues[2]);

            filter = context.Cars.Where(c => c.VehicleMakeName.Contains(make)
                                          && c.VehicleModelName.Contains(model)
                                          && (c.FromYear <= year || c.FromYear == null)
                                          && (c.ToYear >= year || c.ToYear == null));
        }

        return filter
            .GroupBy(c => new { c.SalesCode })
            .Select(g => g.Key.SalesCode).ToList();
    }
}

Move "filter" logic into its own class

There is a slight problem here. The "filter" is a comma separated value string. We can improve this too by making a class called VehicleSearchFilter which will give you a strongly typed way to define the search parameters:

public abstract class VehicleSearchFilter
{
    public static VehicleSearchFilter Parse(string filter)
    {
        if (string.IsNullOrEmpty(filter))
            throw new ArgumentNullException(nameof(filter));

        string[] filterValues = filter.ToUpper().Split(',');
        string make, model;
        int? year;

        if (filterValues.Length > 0)
        {
            make = filterValues[0];
        }

        if (filterValues.Length > 1)
        {
            model = filterValues[1];
        }

        if (filterValues.Length > 2)
        {
            year = int.Parse(filterValues[2]);
        }

        return year == null
            ? new VehicleSearchFilter(make, model)
            : new VehicleSearchFilter(make, model, year);
    }

    public VehicleSearchFilter(string, make string model)
    {
        Make = make;
        Model = model;
    }

    public VehicleSearchFilter(string make, string model, int year)
    {
        Make = make;
        Model = model;
        Year = year;
    }

    public string Make { get; set; }
    public string Model { get; set; }
    public int? Year { get; set; }
}

Now your vehicle search service can use a VehicleSearchFilter object instead of a list of strings:

public class LookupController : ApiController
{
    // GET api/Lookup
    [CacheOutput(ServerTimeSpan = 3600, ClientTimeSpan = 3600)]
    public List<GlassModel> Get(string filter)
    {
        if (string.IsNullOrEmpty(filter)) throw new ArgumentNullException(nameof(filter));

        using (var db = new LookupDbContext())
        {
            IVehicleService service = new VehicleService(db);
            VehicleSearchFilter searchFilter = VehicleSearchFilter.Parse(filter);

            return service.GetGlassModelQuantitesByCountry(searchFilter);
        }
    }
}

And then a few changes to the VehicleService class to use this:

public class VehicleService : IVehicleService
{
    public VehicleService(LookupDbContext context)
    {
        this.context = context;
    }

    private LookupDbContext context;

    public List<GlassModel> GetGlassModelQuantitesByCountry(VehicleSearchFilter filter)
    {
        var salesCodes = GetSalesCodes(filter);
        var queryGlass = context.Countries
            .Where(c => salesCodes.Contains(c.Eurocode))
            .GroupBy(c => new { c.Countrycode })
            .Select(g => new
            {
                g.Key.Countrycode,
                Quantity = g.Sum(p => p.Quantity)
            });

        // build model
        return queryGlass.ToList().Select(x => new GlassModel
        {
            Coordinates = LocationHelper.GetCoordinatesByCountry(x.Countrycode),
            Country = x.Countrycode,
            Quantity = x.Quantity
        }).ToList();
    }

    private GetSalesCodes(VehicleSearchFilter filter)
    {
        var make = filterValues[0];
        IQueryable<Car> filter;

        if (filter.Year == null && filter.Model == null)
        {
            filter = context.Cars.Where(c => c.VehicleMakeName.Contains(filter.Make));
        }
        else if (filter.Model != null)
        {
            filter = context.Cars.Where(c => c.VehicleMakeName.Contains(filter.Make)
                                          && c.VehicleModelName.Contains(filter.Model));
        }
        else
        {
            filter = context.Cars.Where(c => c.VehicleMakeName.Contains(filter.Make)
                                          && c.VehicleModelName.Contains(filter.Model)
                                          && (c.FromYear <= filter.Year || c.FromYear == null)
                                          && (c.ToYear >= filter.Year || c.ToYear == null));
        }

        return filter
            .GroupBy(c => new { c.SalesCode })
            .Select(g => g.Key.SalesCode).ToList();
    }
}

This gives you some compile time checks for your queries, plus it centralizes the parsing of the make, model and year values (which is generic enough I imagine you use it elsewhere too).


Disable button when required inputs are not filled in Currently I'm having a big form, with steps and a lot of complex fields. To make sure the user fills the required inputs and don't make them crazy pushing down the button submit when one required field on step 3 of 6 is no filled in, I decided to disable the button so they can see fast something is not right and check the form back.

My version is working but I'm not satisfied with the result because I'm sure it can be much better.

I simplify my form and I make a fiddle with my code to show the functionality:

You can check my code there.

HTML

  <form action="#myForm">
         Name * <input name="name1" placeholder="required" required>

         Surname * <input name="surname" placeholder="required" required>

         Age <input name="surname" placeholder="not-required">

         <button id="submitBtn" type="submit">Submit</button>
  </form>

Javascript + jQuery

   //This is the first load of is valid, just in case the user gets back again with old inputs
   //if all the required inputs are filled enable the button
   var isValid = true;
     $('input,textarea,select').filter('[required]:visible').each(function() {
      if ( $(this).val() === '' )
         isValid = false;
    });
    if( isValid ) {
       $('#submitBtn').prop('disabled', false);
    } else {
       $('#submitBtn').prop('disabled', true);
    };
    $('#submitBtn').click(function() {
        var isValid = true;
         $('input,textarea,select').filter('[required]:visible').each(function() {
          if ( $(this).val() === '' )
             isValid = false;
        });
        if( isValid ) {
           $("#myForm")[0].submit();
       };

    });

    //When a input is changed check all the inputs and if all are filled, enable the button
     $('input,textarea,select').change(function() {
        var isValid = true;
         $('input,textarea,select').filter('[required]:visible').each(function() {
          if ( $(this).val() === '' )
             isValid = false;
        });
        if( isValid ) {
           $('#submitBtn').prop('disabled', false);
        } else {
           $('#submitBtn').prop('disabled', true);
        };
    });

    //In case the user edits the button with firebug and removes disabled, check
    $('#submitBtn').click(function() {
         var isValid = true;
         $('input,textarea,select').filter('[required]:visible').each(function() {
          if ( $(this).val() === '' )
             isValid = false;
        });
        if( isValid ) {
           $("#myForm")[0].submit();
       };
    });

Sure you can DRYfy you code in several ways.

The most obvious point is that you're using 3 times the almost identical sequence of statements, so it should be factorized.

var isValid = true;
 $('input,textarea,select').filter('[required]:visible').each(function() {
  if ( $(this).val() === '' )
     isValid = false;
});
// then do something with "isValid"

Having isolated the sequence, we can already both simplify and improve its "core":

$(this).val() can be simpler and faster: this.value
but as pointed by @Tushar, it should be secured: this.value.trim();
on the other hand, yes we might stop looping as soon as false (returning false rather than with break;, since each() is not a loop), but there can never be a significative lot of inputs so personally I'll give up, for the sake of readability
and finally the whole condition $(this).val() === '' can be yet simplified: !this.value.trim()

Now it's time to look at how to really factorize, i.e. make this sequence usable each time we need it to work.

But at the same time we may also think about how to make it of more general use. The idea here is to be able to work independently (while automatically) upon all or some of several forms.
It may be achieved by wrapping the sequence in a handler:

fired on any input element
looking for valid state of all input elements in its own form

So the whole sequence becomes a function like this:

function checkForm() {
  // here, "this" is an input element
  var isValidForm = true;
  $(this.form).find(':input[required]:visible').each(function() {
    if (!this.value.trim()) {
      isValidForm = false;
    }
  });
  // then do something with "isValidForm"... not stated yet
}

At this step we must decide how to make this handler be launched.
It worth mention that change is not a good choice, because it fires only when the element value had changed AND the element lost focus.
So for example, based on a full valid form, if user empties a required input the submit button remains enabled till navigated to another element!

It's why I suggest using keyup event instead (nevertheless if checkboxes or radios should also be involved, you'll have to also use change event).

For the sake of simple usage, I suggest affecting a distinguishing class to the submit button of each form you want to be involved, say monitored-btn.
So the way to make monitoring active will be:

$('.monitored-btn').closest('form').find(':input[required]:visible').keyup(checkForm);

This way, we know that our handler will be fired each time an element value changes.
So it's time to add what we want the handler to do depending on the form state, so it becomes:

function checkForm() {
  // here, "this" is an input element
  var isValidForm = true;
  $(this.form).find(':input[required]:visible').each(function() {
    if (!this.value.trim()) {
      isValidForm = false;
    }
  });
  $(this.form).find('.monitored-btn').prop('disabled', !isValidForm);
}

Now, each time an element value changes the involved submit button is enabled or not depending on the whole form state.
But when page was just loaded, the buttons are always all enabled regardless of anything else.
It's why we must immediately fire the handler as soon as it has been bound:

$('.monitored-btn').closest('form').find(':input[required]:visible')
  // bind the handler
  .keyup(checkForm)
  // immediately fire it to initialize buttons state
  .keyup();

Last point, regarding your care of user-hacking with Firebug (a bit strange, from my point of view, since if user can suppress the disabled attribute he can also do almost anything he wants, including directly submit through the console).

A simple way to also monitor that is to fire the same handler on form submit event, with two slight changes:

the handler expects this to be an input element, so we must call it like if it was a simple function (not a handler): checkForm.apply($(this).find(':input')[0]);
apart from enabling/desabling button, the handler should now return isValidForm, so we can use it to prevent defaut

It actually results in this final whole version:

function checkForm() {
  // here, "this" is an input element
  var isValidForm = true;
  $(this.form).find(':input[required]:visible').each(function() {
    if (!this.value.trim()) {
      isValidForm = false;
    }
  });
  $(this.form).find('.monitored-btn').prop('disabled', !isValidForm);
  return isValidForm;
}

$('.monitored-btn').closest('form')
  // indirectly bind the handler to form
  .submit(function() {
    return checkForm.apply($(this).find(':input')[0]);
  })
  // look for input elements
  .find(':input[required]:visible')
  // bind the handler to input elements
  .keyup(checkForm)
  // immediately fire it to initialize buttons state
  .keyup();

Here is a working example where I used two forms with distinct "required" exigences:

const inputSelector = ':input[required]:visible';
function checkForm() {
  // here, "this" is an input element
  var isValidForm = true;
  $(this.form).find(inputSelector).each(function() {
    if (!this.value.trim()) {
      isValidForm = false;
    }
  });
  $(this.form).find('.monitored-btn').prop('disabled', !isValidForm);
  return isValidForm;
}
$('.monitored-btn').closest('form')
  // in a user hacked to remove "disabled" attribute, also monitor the submit event
  .submit(function() {
    // launch checkForm for the first encountered input,
    // use its return value to prevent default if form is not valid
    return checkForm.apply($(this).find(':input')[0]);
  })
  .find(inputSelector).keyup(checkForm).keyup();
form {
  border: 1px solid #000;
  margin: 10px;
}

  <form action="#myForm">
         Name * <input name="myForm-name1" placeholder="required" required>

         Surname * <input name="myForm-surname" placeholder="required" required>

         Age <input name="myForm-surname" placeholder="not-required">

         <button id="myForm-submitBtn" class="monitored-btn" type="submit">Submit</button>
  </form>
  <form action="#otherForm">
         Name * <input name="otherForm-name1" placeholder="required" required>

         Surname * <input name="otherForm-surname" placeholder="not-required">

         Age <input name="otherForm-surname" placeholder="required" required>

         <button id="otherForm-submitBtn" class="monitored-btn" type="submit">Submit</button>
  </form>


Find the largest odd number Ask user to input 10 integers and then print the largest odd number that was entered. If no odd number was entered, print a message to that effect.

The above exercise is from chapter 2 of Computation and Programming Using Python by John Guttag. So far, the book has covered variable assignment, print function, conditional branching, and loops. The book is about problem solving and uses Python only as a tool to solve problems and it is meant to be used with Python 2.x. I need to know if my solution to the exercise can be "straightened up" in any way.

numbers_entered = []

for _ in range(10):
    number = raw_input('Enter an integer: ')
    if number.isdigit():
        number = int(number)
        numbers_entered.append(number)
    else:
        print 'That was not an integer!'

odd_numbers = []

for item in numbers_entered:
    if item % 2 != 0:
        odd_numbers.append(item)

if odd_numbers:
    print 'The largest odd number entered was %s.' % str(max(
            odd_numbers))
else:
    print 'No odd number was entered.'

Bug

At the end of the first loop,
how many numbers will be in numbers_entered?
10? Hopefully.
In reality: 10 minus invalid entries,
and I'm not sure that was your intention.

This way the invalid entries will be skipped, and there will always be 10 entries in list:

while True:
    number = raw_input('Enter an integer: ')
    if number.isdigit():
        number = int(number)
        numbers_entered.append(number)
        break
    else:
        print 'That was not an integer!'

Handling invalid input

The user input should be valid,
invalid inputs should be the exception.
So you can simplify the handling of invalid user inputs using exceptions:

try:
    number = int(raw_input('Enter an integer: '))
    numbers_entered.append(number)
except ValueError:
    print 'That was not an integer!'

See also the glossary, and search for the word "forgiveness" in it.

Using list comprehensions

List comprehensions are awesome.
The loops could be replaced by list comprehensions
(with the help functions), for example:

def read_int():
    while True:
        try:
            return int(raw_input('Enter an integer: '))
        except ValueError:
            print 'That was not an integer!'

numbers = [read_int() for _ in range(10)]

odd_numbers = [item for item in numbers if item % 2]

if odd_numbers:
    print 'The largest odd number entered was {}.'.format(max(odd_numbers))
else:
    print 'No odd number was entered.'

Prefer .format(...) for formatting

Instead of formatting with %,
the new preferred way is using .format(...):

print 'The largest odd number entered was {}.'.format(max(odd_numbers))

Notice that there's no need to call str(...) when using this style.

Readability

This statement would have been better on a single line:

print 'The largest odd number entered was %s.' % str(max(
        odd_numbers))


Splitting an integer to its digits I want to  split an integer of type unsigned long long to its digits.
Any comments and suggestions are always welcome.
#include <iostream>
#include <string>

#include <vector>

using namespace std;

vector<unsigned short> IntegerDigits(unsigned long long input)
{
    vector<unsigned short> output;
    const unsigned short n = log10(ULLONG_MAX);
    unsigned long long divisor = pow(10, n);
    bool leadingZero = true;

    for (int i = 0; i < n + 1; i++)
    {
        unsigned short digit = input / divisor % 10;
        if (!leadingZero || digit != 0)
        {
            output.push_back(digit);
            leadingZero = false;
        }
        divisor /= 10;
    }

    return output;
}

void main()
{
    vector<unsigned short> output = IntegerDigits(ULLONG_MAX);

    cout << ULLONG_MAX << ": [";
    for (auto y : output)
        cout << y << ", ";
    cout << "\b\b]";

    cout << ""<<endl;

}

Avoid mixing floating point and integer arithmetic
As mentioned by greybeard, there is a potential problem here:
const unsigned short n = log10(ULLONG_MAX);

ULLONG_MAX is larger than can be exactly represented by a double. This means the result might not be what you expect. The same goes for pow(10, n). While you can compensate for it, it is better to find a way to calculate the length of a number without using floating point math.
Keep it simple
Unless performance is a big concern, keep it simple. You don't have to know the number of digits up front if you push trailing digits to the front of the vector, like so:
vector<unsigned short> IntegerDigits(unsigned long long input)
{
    vector<unsigned short> output;

    while (input)
    {
        output.insert(output.begin(), input % 10);
        input /= 10;
    }

    // Handle input being equal to 0
    if (output.empty())
    {
        output.push_back(0);
    }

    return output;
}

Pushing to the front of a std::vector is less efficient, but on the other hand you don't need the double<->int conversions, and you don't need to handle the leading zeros inside the loop.
Avoid using std::endl
Prefer using "\n" over std::endl, the latter is equivalent to the former, but also forces a flush of the output, which can be bad for performance.
Avoid backspaces in the output
You used a neat trick to get rid of the last comma without having to have extra logic inside the for-loop in main(). However, consider that the output might not just be for human consumption, but is written to a file and/or is parsed by another program. In that case, the \b characters are probably unexpected and might cause problems.


Re-arranging an obfuscated address I'm getting address (physical address, not digital) input that's obfuscated, and looks like the following:

  The plaintext version:

'39 Jerrabomberra Ave. Narrabundah Canberra 2604 Australia'

  The obfuscated version:

['39 Jerrabomberra Ave., Narrabundah', 'Canberra', ' ', '2604', ', ', 'Australia', '39 Jerrabomberra Ave., Narrabundah', 'Canberra 2604, ', 'Australia']

Usually the obfuscation is simple duplication and rearranging, which my script catches, however there are a few edge cases that are missed, which I'm working on catching.

However, my solution feels like it could be simplified.

The logic follows this order:

Join the array into one long string with a space as the 'glue' character.
Use re.sub to find all commas and remove them.
Split by space
Add each non-empty component to the components array if it is not already in there.
Join the components together.

import re

...

address = fooGetAddress(foo[bar]) #returns an array
address_components = []
for component in re.sub(",", "", " ".join(address)).split(" "):
    if component not in address_components and component is not "":
        address_components.append(component)
address = " ".join(address_components)

Not bad.  However, we can do away with if component not in address_components and component is not "".

A better way to check if component not in address_components would be to use collections.OrderedDict:

  An OrderedDict is a dict that remembers the order that keys were first inserted. If a new entry overwrites an existing entry, the original insertion position is left unchanged.

That's exactly what we want.  (Well, almost exactly.  What we really want is an ordered set, but we can just use the keys of an OrderedDict and ignore the values.)

We can eliminate the need for component is not "" by using str.split() instead of str.split(" "):

  If sep is not specified or is None, a different splitting algorithm is applied: runs of consecutive whitespace are regarded as a single separator, and the result will contain no empty strings at the start or end if the string has leading or trailing whitespace. Consequently, splitting an empty string or a string consisting of just whitespace with a None separator returns [].

To get rid of the commas, you don't need a regular expression.  str.replace() will do.

It's bad practice to use the same variable (address) for two different purposes, especially when the type changes (from a list of strings to a string).

With those changes, we can write the solution as just a single expression.

from collections import OrderedDict

obfuscated_address = …
address = ' '.join(
    OrderedDict(
        (component, None) for component in
        ' '.join(obfuscated_address).replace(',', '').split()
    ).keys()
)


Change Task Edit form of workflow in SharePoint 2010 I want to change the task edit form of the workflow when an approver wants to approve/reject a task. Please review and suggest improvements or any other possible solution with an explanation.

 public void createTask1_MethodInvoking(object sender, EventArgs e)
        {
            createTask1_TaskId1 = Guid.NewGuid();
            createTask1_TaskProperties1.Title = "Test Title";
            createTask1_TaskProperties1.AssignedTo = "i:0#.w|directory\approver";
            var taskList = workflowProperties.TaskList;
            var wfContentType = taskList.ContentTypes["Workflow Task"];
            TaskTitle = workflowProperties.Item["Title"].ToString();
            wfContentType.DisplayFormUrl = "_Layouts/SharePointProject4/ApplicationPage1.aspx";
            wfContentType.EditFormUrl = wfContentType.DisplayFormUrl;
            wfContentType.Update();
            taskList.NavigateForFormsPages = false;
            taskList.Update();
        }

Give your code some room to breathe.

Empty space costs nothing in C#, so add some gaps to group together similar concepts.

 public void createTask1_MethodInvoking(object sender, EventArgs e)
    {
        createTask1_TaskId1 = Guid.NewGuid();
        createTask1_TaskProperties1.Title = "Test Title";
        createTask1_TaskProperties1.AssignedTo = "i:0#.w|directory\approver";

        var taskList = workflowProperties.TaskList;
        var wfContentType = taskList.ContentTypes["Workflow Task"];

        TaskTitle = workflowProperties.Item["Title"].ToString();

        wfContentType.DisplayFormUrl = "_Layouts/SharePointProject4/ApplicationPage1.aspx";
        wfContentType.EditFormUrl = wfContentType.DisplayFormUrl;
        wfContentType.Update();

        taskList.NavigateForFormsPages = false;
        taskList.Update();
    }

Secondly, when naming variables, be explicit. There's no need to abbreviate anything, once again, extra characters are free.

  public void createTask1_MethodInvoking(object sender, EventArgs e)
    {
        createTask1_TaskId1 = Guid.NewGuid();
        createTask1_TaskProperties1.Title = "Test Title";
        createTask1_TaskProperties1.AssignedTo = "i:0#.w|directory\approver";

        var taskList = workflowProperties.TaskList;
        var workFlowContentType = taskList.ContentTypes["Workflow Task"];

        TaskTitle = workflowProperties.Item["Title"].ToString();

        workFlowContentType.DisplayFormUrl = "_Layouts/SharePointProject4/ApplicationPage1.aspx";
        workFlowContentType.EditFormUrl = wfContentType.DisplayFormUrl;
        workFlowContentType.Update();

        taskList.NavigateForFormsPages = false;
        taskList.Update();
    }

Next, onto design. You've named a task createTask1, which is concerning, because that means it's possible you've created a lot of these with different numbers. Pop them in a collection named createTasks instead and try to refactor your MethodInvoking event handler to be abstracted across all createTasks. If there's something about createTask1 that means the event handler logic cannot be applied to the other createTask instances, then you should name createTask1 based on this unique property.

It's hard to tell what this method handler is meant to be doing since it's just named after the event that calls it. Give it a better name, or extract this functionality into its own method with a sensible name and call it from createTask1_MethodInvoking.

Lastly, I'm seeing an absolute ton of magic strings, which make it very hard indeed to sensibly refactor, because you'll have to change things in so many places, and they also hamper the understandability of the code. Make pretty much all of those strings into constant fields with sensible names.


Making my User model class code more modular, and correct This is my first User model I created.

And I wanted to know if there are any good ways I can improve my code? It works with sessions, database and cookies.

I feel like some parts are somewhat written in a weird way, like in my constructor.

I would like to have my code as modular as possible, and maybe if there is anything to improve on when it comes to readability. It's understandable to me, because I wrote it, but it gets slightly complex.

class User
{
    private $db;
    private $req;
    private $data;

    public function __construct(Database $db, RequestHandler $req, $user = null)
    {
        $this->db  = $db;
        $this->req = $req;
        $sid       = $req->session('uid');
        $cHash     = $req->cookie('shash');

        if ($sid && !$user && !$cHash && !$this->data = $this->search($sid)) {
            # if only session user id is set AND user cannot be found

            unset($_SESSION['uid']);
        } elseif (!$sid && $user && !$this->data = $this->search($user)) {
            throw new Exception('User, ' . $user .  ', does not exist.');
        } elseif (!$sid && !$user && $cHash) {
            # if only shash (remember) cookie is present

            $sql  = 'SELECT * FROM sessions WHERE hash = ?';
            $data = $this->db->query($sql, $cHash);

            if ($data->rowCount() > 0) {
                $id = $_SESSION['uid'] = $data->fetch()->user_id;
                $this->data = $this->search($id);
            }
        }
    }

    public function create($fname, $lname, $email, $pw)
    {
        $sql = 'INSERT INTO users (firstname, lastname, email, password) VALUES(?, ?, ?, ?)';

        $this->db->query($sql, [$fname, $lname, $email, $pw]);
    }

    public function search($user)
    {
        $col  = is_numeric($user) ? 'id' : 'email';
        $sql  = 'SELECT * FROM users WHERE ' . $col . ' = ?';
        $data = $this->db->query($sql, $user);

        if ($data->rowCount() > 0) {
            return $data->fetch();
        }

        return false;
    }

    public function login($email, $pw, $remember = false)
    {
        if ($data = $this->search($email)) {
            $pwHasher = new PasswordHash(8, false);
            $pw       = $pwHasher->CheckPassword($pw, $data->password);

            if ($pw) {
                # user succussfully authenticated
                $this->data      = $data; //load data in entire class
                $_SESSION['uid'] = $data->id;

                if ($remember == true) {
                    $sql  = 'SELECT hash FROM sessions WHERE user_id = ?';
                    $data = $this->db->query($sql, $data->id);

                    if ($data->rowCount() === 0) {
                        $hash = CryptoCharGen::alnum(60);
                        $sql  = 'INSERT INTO sessions (user_id, hash) VALUES(?, ?)';

                        $this->db->query($sql, [$this->data->id, $hash]);

                        setcookie('shash', $hash, time() + 60 * 60 * 24 * 7);
                    }
                }

                return true;
            }
        }

        return false;
    }

    public function logout()
    {
        $sql = 'DELETE FROM sessions WHERE user_id = ?';

        $this->db->query($sql, $this->data->id);

        unset($_SESSION['uid']);

        setcookie('shash', '', time() - 1);

        session_regenerate_id(true);
    }

    public function getData()
    {
        return $this->data;
    }

    public function isLoggedIn()
    {
        return isset($this->data);
    }
}

There are a couple of things that I would personally change about the code, at the moment the model is doing far too much. It's pulling in cookies, request parameters and accessing the database. That's not the responsibility of the model, any domain logic should be separated from the models themselves.

First I would recommend moving the session and cookie related code somewhere more suitable, like a Filter, which can be ran before an action in a Controller is executed (if we're using the MVC pattern here).

Let's call our Filter class UserAuthFilter, before each request is routed to the relevant controller this class will intercept the request and check if the user is currently signed in.

class UserAuthFilter extends Filter {

    public function beforeAction(RequestHandler $request) {
        $uid = $request->session('uid');
        $sessionHash = $request->cookie('shash');

        //check if these variables are connected to a valid login
        if(/*we are logged in*/) {
            $user = ...;
            $request->set('user', $user);
        }

        return true; // continue with execution
    }
}

Second, I would recommend having your User model be nothing more than a representation of data. So no domain logic in the User class, just data. Anything database related we will move into a new class, let's call it UserRepository.

public class UserRepository {

    private $db;

    public function __construct(Database $db) {
        $this->db = $db;
    }

    public function create($fname, $lname, $email, $password) {
        // create user
    }

    public function search($searchTerm) {
        // search for $searchTerm
    }

    public function getUserByCredentials($username, $password) {
        // find a user with the gieven username and password or return null/false
    }
}

Now, the last thing left to refactor is the authentication side of things. To do this we'll create another new class, UserAuthService, which will handle authentication and session management.

public class UserAuthService {

    private $userRepo;

    public function __construct(UserRepository $userRepo) {
        $this->userRepo = $userRepo;
    }

    public function authenticate($username, $password) {
        $user = $this->userRepo->getUserByCredentials($username, $password);
        return $user === false ? false : $user;
    }

    public function logout(User $user) {
        // delete from sessions table where uid = $user->id
    }
}

Then instead of inserting into the session table from the UserAuthService on a successful login, from within your controller you check the credentials with a call to $userAuthService->authenticate(...) and if you get a valid result back, then you insert into the session table.

That's about all I can think of besides a couple of small improvements:

Give your classes meaningful names. RequestHandler doesn't seem like a suitable name given the calls you're making to it. Perhaps just Request would be better.
Try to keep your constructors as simple as possible, do you really need to be making calls to the database when a User object is created?

Hope this helps.


Return the first recurring character in a string The task:

  Given a string, return the first recurring character in it, or null if
  there is no recurring character.

  For example, given the string "acbbac", return "b". Given the string
  "abcdef", return null.

My imperative solution:

function getFirstRecurringChar(str) {
  let i = 0;
  const len = str.length;
  const letters = {};
  while(i < len && !letters[str[i]]) { letters[str[i++]] = true; }
  return i < len ? str[i] : null;
};

console.log(getFirstRecurringChar("acbbac"));

My functional solution:

const getFirstRecurringChar2 = str => {
  const letters = {};
  let lastLetter;
  return Array.from(str).some(x => {
    if(letters[x]) {
      lastLetter = x;
      return true;
    }
    letters[x] = true;
    return false;
  }) ? lastLetter : null;
};

console.log(getFirstRecurringChar2("abcdef"));

The problem is similar to your other "two of something" problems, such as Palindrome and Find the elements that appear only once.  As in those problems, a Set is the appropriate data structure.

The main difference is that early exit is possible (when two of something is found) so we'd rather not be inside of a reduce.

const firstRepeated = s => {
  const seen=new Set();
  for (var c of s) {
      if (seen.has(c)) return c;
      else seen.add(c);
  }
  return null;
}


Invoice receipt markup Prompted by a meta answer, here is an HTML page I'm deliberating over in JSBIN and I would like your review, please.

I am not looking to implement the logic, just please focus on HTML & CSS.
Are tables a good candidate to mark up purchase items? I've seen this implemented as a list in each column, which I thought was worse.
How can I best add markup and style to the final total?

There is a known issue with the CSS border. I'm not asking for you to fix that, but I'm telling you because there is a comment in my code that could possibly make it sound broken. Any other good tips or examples would be great.

td, th { text-align: right; }
tr th:first-child, tr td:first-child { text-align: left; }
tbody tr:last-child td { font-weight: bold;  }
table { border-collapse: collapse; }
tbody tr:last-child { border-top: thick black solid; }

      Description
      Quantity
      Rate
      Amount

      Moon cakes
      5
      100
      500

      Super biscuits
      1
      500
      500

 Total value

 1000

Using a table here is the right choice, however I would put the total into a <tfoot> group where you can address its formatting needs specifically. In addition do use colspan="2" to collapse columns in the total, as well.


Redux actions inside a container for a calculator I used to import all the actions in the container's argument which looks messy (I put a comment below where I do it in that way). I want to know if there is a better approach when using actions with react-redux. Should I use alias, and if I use alias is there any impact on the performance?

import React from 'react';
import Proptypes from 'prop-types';
import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';

import {
  selectNumber,
  selectOperator,
  solverOperator,
  initialize,
  addDecimal,
  clearEntry,
  changeSign,
} from '../actions/index';

const ButtonsContainer = ({ selectNumber, selectOperator, solverOperator, initialize, addDecimal, clearEntry, changeSign, }) => (  // HERE I PUT ALL THE ACTIONS AS ARGUMENT
  <React.Fragment>
    <button value="AC" onClick={() => initialize()} className="buttonStyle ac" type="submit">AC</button>
    <button value="CE" onClick={() => clearEntry()} className="buttonStyle ce" type="submit">CE</button>
    <button value="CHANGE" onClick={e => changeSign(e.target)} className="buttonStyle posneg" type="submit">+/-</button>
    <button value="/" onClick={e => selectOperator(e.target)} className="buttonStyle divide" type="submit">/</button>
    <button value="7" onClick={e => selectNumber(e.target)} className="buttonStyle seven" type="submit">7</button>
    <button value="8" onClick={e => selectNumber(e.target)} className="buttonStyle eight" type="submit">8</button>
    <button value="9" onClick={e => selectNumber(e.target)} className="buttonStyle nine" type="submit">9</button>
    <button value="*" onClick={e => selectOperator(e.target)} className="buttonStyle multiply" type="submit">X</button>
    <button value="4" onClick={e => selectNumber(e.target)} className="buttonStyle four" type="submit">4</button>
    <button value="5" onClick={e => selectNumber(e.target)} className="buttonStyle five" type="submit">5</button>
    <button value="6" onClick={e => selectNumber(e.target)} className="buttonStyle six" type="submit">6</button>
    <button value="-" onClick={e => selectOperator(e.target)} className="buttonStyle minus" type="submit">-</button>
    <button value="1" onClick={e => selectNumber(e.target)} className="buttonStyle one" type="submit">1</button>
    <button value="2" onClick={e => selectNumber(e.target)} className="buttonStyle two" type="submit">2</button>
    <button value="3" onClick={e => selectNumber(e.target)} className="buttonStyle three" type="submit">3</button>
    <button value="+" onClick={e => selectOperator(e.target)} className="buttonStyle plus" type="submit">+</button>
    <button value="0" onClick={e => selectNumber(e.target)} className="buttonStyle zero" type="submit">0</button>
    <button value="." onClick={e => addDecimal(e.target)} className="buttonStyle dot" dangerouslySetInnerHTML={{ __html: '&middot' }} type="submit" />
    <button value="=" onClick={e => solverOperator(e.target)} className="buttonStyle equal" type="submit">=</button>
  </React.Fragment>
);

ButtonsContainer.propTypes = {
  selectNumber: Proptypes.func.isRequired,
  selectOperator: Proptypes.func.isRequired,
  solverOperator: Proptypes.func.isRequired,
  initialize: Proptypes.func.isRequired,
  addDecimal: Proptypes.func.isRequired,
  clearEntry: Proptypes.func.isRequired,
  changeSign: Proptypes.func.isRequired,
};

const mapDispatchToProps = dispatch => bindActionCreators({
  selectNumber,
  selectOperator,
  solverOperator,
  initialize,
  addDecimal,
  clearEntry,
  changeSign,
}, dispatch);

I would try to extract some of the common functionality into reusable components as follows. The NumericButton and OperatorButton components could either receive selectNumber and selectOperator as props or the components could be connected components with the select* props coming from mapDispatchToProps. The connected component method eliminates the need to pass the prop in every component instance in ButtonContainer, but increases the number of connected components thus adding some complexity.

const CalculatorButton = ({ children, className, onClick, value}) => {
  return <button value={value} onClick={e => onClick(e.target)} className={`buttonStyle ${className || ''}`} type="submit">{children || value}</button>;
};

const NumericButton = ({ className, value }) => {
  return <CalculatorButton className={className} onClick={selectNumber} value={value} />;
};

const OperatorButton = ({ className, value }) => {
  return <CalculatorButton className={className} onClick={selectOperator} value={value} />
};

const ButtonsContainer = () => (
  <React.Fragment>
    <CalculatorButton className="ac" onClick={() => initialize()} value="AC" />
    <CalculatorButton className="ce" onClick={() => clearEntry()} value="CE" />
    <CalculatorButton className="posneg" onClick={changeSign} value="CHANGE">{'+/-'}</CalculatorButton>
    <OperatorButton className="divide" value="/" />
    <NumericButton className="seven" value="7" />
    <NumericButton className="eight" value="8" />
    <NumericButton className="nine" value="9" />
    {/* etc. */}
  </React.Fragment>
);


Trello list scraper with data visualization - Monthly food expenses For the last couple of months I've been working on a python script that pulls data from a specific Trello list and sums up the numeric values by list (lists are split up into months). I've worked on making my code pythonic, basic functionality, separating functionality into distinct functions.
It currently only works with a specific list and board.
Things I plan to work on is; better error handling with network connections and introducing unit testing. Two things I have little experience with.
EDIT: the keys.txt file is formatted as follows

token=token
api_key=key

#!/usr/bin/python
from trollop import TrelloConnection

import logging
from math import ceil
from time import strftime

import seaborn as sns
import matplotlib.pyplot as plt

# Lots of issues with Python3. Lots of unicode, string errors, Just switched to
# py2. should try to use dicts for {name: cost} and to  practice using dicts

# TODO: Data Visualization, Error/exception handling
# TODO: clean up code, get feedback on reddit, maybe use args for spec params
# TODO: set up cron job to run every month
k = list()
with open('keys.txt', 'r') as keys:
    for line in keys:
        tmp = line.split('=')[1]
        tmp = tmp.rstrip()
        k.append(tmp)
token = k[0]
api_key = k[1]

# idBoard': '577b17583e5d17ee55b20e44',
# idList': '577b17583e5d17ee55b20e45',
# Set up basic logging
logging.basicConfig(format='%(levelname)s %(message)s',
                    level=logging.INFO, filename='DEBUG.log',
                    filemode='w')
# Establish connection
conn = TrelloConnection(api_key, token)
MONTHS = ['Jan', 'Feb', 'March', 'April', 'May', 'June', 'July', 'Aug',
          'Sept', 'Oct', 'Nov', 'Dec']

logging.info("Iterating through boards...")

def get_total_per_month(month, board_list):
    costs = 0.0
    month = month.lower()
    for lst in board_list:
        if month in lst.name.lower():
            for crd in lst.cards:
                costs += float(crd.name.split('-')[1])
    return ceil(costs)

def first_of_the_month():
    day = strftime("%d")
    if '1' is day:
        # TODO: check to see if it's a new month and add a list
        pass

def get_yearly_average(totals):
    sum = 0.0
    count = 0
    for month in totals:
        if month != 0.0:
            count = count + 1
            sum += month
            # print month
    year_average = sum / count
    print 'year ave ' + str(year_average)
    return year_average

def plot(totals, average):
    sns.set(style='white', font_scale=1.5)
    plt.title('Monthly Food Expenses')
    plt.xlabel('Months')
    plt.ylabel('Pesos')
    sns.barplot(x=MONTHS, y=totals)
    plt.show()

def main():
    costs = list()
    names = list()

    board = conn.get_board('BE89pW61')
    totals = [get_total_per_month(month, board.lists) for month in MONTHS]
    print totals
    average = get_yearly_average(totals)
    logging.info(totals)
    logging.debug('Board list: {}'.format(board.lists))
    plot(totals, average)
if __name__ == '__main__':
    main()

Remove code from the global scope. You should try to keep global scope to just functions and classes. This is as adding data here means any function can change it, and so you can no-longer trust it's state.

The only time I use globals that contain data, is when I use a constant. Take MONTHS. Which is a good global variable.

I'd add your settings as a global constant too. You should probably use a dictionary, as then you don't have to use dodgy indexing. But, using a dictionary means that you'd have to use SETTINGS['token'], rather than SETTINGS.token. And so I'd make a new class that has this sugar:

class FrozenDict(object):
    def __init__(self, *args, **kwargs):
        # Hack to bypass `__setattr__`.
        self.__dict__['d'] = dict(*args, **kwargs)

    def __getattr__(self, key):
        return self.d[key]

    def __setattr__(self, key, val):
        raise TypeError('FrozenDict does not support setting attributes.')

    def __delattr__(self, key, val):
        raise TypeError('FrozenDict does not support deleting attributes.')

After this you may want to put getting settings into it's own function. As you wrote:

  TODO: clean up code, get feedback on reddit, maybe use args for spec params

This new function should allow you to use both a file and take arguments  from the command line. I won't go into how to do this, as Reddit probably did. But I'd use argparse. But back to your code. I'd change the construction of the list to be entirely in the with. As with statements aren't in a different scope. And I would use a list comprehension to build the list, rather than using append. This can get you:

def read_settings():
    with open('keys.txt', 'r') as keys:
        k = [line.split('=')[1].rstrip() for line in keys]
        token = k[0]
        api_key = k[1]
    return FrozenDict({
        'token': k[0],
        'api_key': k[1],
        'board': 'BE89pW61'
    })

Nice and simple. I don't know what keys.txt is structured like, but I would recommend that you instead format it so it's:

token=...
api_key=...
board=BE89pW61

Which would allow you to instead use a dictionary comprehension. And would allow you to not be so ridged on how the settings are provided. Using the second argument to str.split. And taking advantage of dict you could get:

def read_settings():
    with open('keys.txt', 'r') as keys:
        d = dict(line.split('=', 1) for line in keys)
    return FrozenDict(d)

After this I'd clean up both get_total_per_month and get_yearly_average. Instead of performing the addition yourself you can instead use sum. And to count the amount of items in an array you can use len. This allows you to change get_yearly_average to a list comprehension, and then just a sum divided by its length. And allows you to change get_total_per_month to a comprehension, passed to sum. I'd also remove the ceil in get_total_per_month as you should change the data when outputting not in it's internal state. And leads to \$\pm12\$ accuracy, as if every month is \$0.01\$ you'll say they're \$1\$, and \$12 \dot{} 0.01 = 0.12\$, but \$12 \dot{} 1 = 12\$. Which is absurdly wrong.

Finally I'd remove your prints as you're using logging. It makes no sense to use print and Pythons logging library to log the programs state. You should only use print as a means to interact with the user, such as asking if they want to use keys.txt as their settings file.

This can leave you with:

#!/usr/bin/python

# Lots of issues with Python3. Lots of unicode, string errors, Just switched to
# py2. should try to use dicts for {name: cost} and to  practice using dicts

# TODO: Data Visualization, Error/exception handling
# TODO: clean up code, get feedback on reddit, maybe use args for spec params
# TODO: set up cron job to run every month

import logging

from trollop import TrelloConnection
import seaborn as sns
import matplotlib.pyplot as plt

MONTHS = ['Jan', 'Feb', 'March', 'April', 'May', 'June', 'July', 'Aug',
          'Sept', 'Oct', 'Nov', 'Dec']

class FrozenDict(object):
    def __init__(self, *args, **kwargs):
        self.__dict__['d'] = dict(*args, **kwargs)

    def __getattr__(self, key):
        return self.d[key]

    def __setattr__(self, key, val):
        raise TypeError('FrozenDict does not support setting attributes.')

    def __delattr__(self, key, val):
        raise TypeError('FrozenDict does not support deleting attributes.')

def read_settings():
    with open('keys.txt', 'r') as keys:
        k = [line.split('=')[1].rstrip() for line in keys]
        token = k[0]
        api_key = k[1]
    return FrozenDict({
        'token': k[0],
        'api_key': k[1],
        'board': 'BE89pW61'
    })

def get_total_per_month(month, board_list):
    month = month.lower()
    return sum(
        float(crd.name.split('-')[1])
        for lst in board_list
        if month in lst.name.lower()
        for crd in lst.cards
    )

def get_yearly_average(totals):
    totals = [t for t in totals if t != 0]
    return sum(totals) / len(totals)

def plot(totals, average):
    sns.set(style='white', font_scale=1.5)
    plt.title('Monthly Food Expenses')
    plt.xlabel('Months')
    plt.ylabel('Pesos')
    sns.barplot(x=MONTHS, y=totals)
    plt.show()

def main():
    # Establish connection
    conn = TrelloConnection(SETTINGS.api_key, SETTINGS.token)
    board = conn.get_board(SETTINGS.board)
    totals = [get_total_per_month(month, board.lists) for month in MONTHS]
    average = get_yearly_average(totals)
    logging.info(totals)
    logging.debug('Board list: {}'.format(board.lists))
    plot(totals, average)

if __name__ == '__main__':
    SETTINGS = read_settings()

    # Set up basic logging
    logging.basicConfig(format='%(levelname)s %(message)s',
                        level=logging.INFO, filename='DEBUG.log',
                        filemode='w')

    logging.info("Iterating through boards...")
    main()


Convert Decimal to Fraction This code below converts number from decimal to fraction. I don't know if there's a better way or better methods to do this job so any advice would be appreciated.
const toFraction = (inputNumber) => {
  // Only two digits after decimal point
  inputNumber = inputNumber.toFixed(2);

  // destructure number
  let [int, dec] = inputNumber
    .toString()
    .split(".")
    .map((el) => parseInt(el));

  // Round it up to nearst multiple of 5
  dec = Math.ceil(dec / 5) * 5;

  // Keep Dividing until it's impossible
  const divide = [9, 8, 7, 6, 5, 4, 3, 2];
  let hundred = 100;
  let notOver = true;
  while (notOver) {
    notOver = false;
    for (let num of divide) {
      if (dec % num === 0 && hundred % num === 0) {
        dec /= num;
        hundred /= num;
        notOver = true;
      }
    }
  }
  return int === 0 ? `${dec}${hundred}` : `${int} ${dec}/${hundred}`;
};

You can remove the toString() bit from here:
  let [int, dec] = inputNumber
    .toString()
    .split(".")
    .map((el) => parseInt(el));

toFixed returns a string. So, there is no need to call toString again.

The calculation part:
If the decimal part is 12, your output is 3/25. How do you get this? You get the greatest common divisor between 12 and 100 and divide both of them with it. GCD of 12 and 100 is 4. So, (12/4) / (100/4)
There's already a simple recursive implementation of euclidean algorithm for that:
function gcd(a, b)
    if b = 0
        return a
    else
        return gcd(b, a mod b)

You can get the precision required as an option parameter which defaults to a predefined MAX_PRECISION. Get the GCD between the dec and 10 precision (eg: 100 for precision = 2)

const MAX_PRECISION = 5

function toFraction(inputNumber, precision = MAX_PRECISION) {
  let [int, dec] = inputNumber
                      .toFixed(precision)
                      .split(".")
                      .map(n => +n)

  const powerOf10 = 10 ** precision,
        gcd = getGCD(dec, powerOf10),
        fraction = `${dec/gcd}/${powerOf10/gcd}`;

  return int ? `${int} ${fraction}` : fraction
};

function getGCD(a, b) {
  if (!b) return a;

  return getGCD(b, a % b);
};

console.log( toFraction(1.14)   );
console.log( toFraction(5.71)   );
console.log( toFraction(3.34)   );
console.log( toFraction(5.1044) );
console.log( toFraction(0.67)   );
console.log( toFraction(0.84)   );


Solving for equilibrium index in an array I did a sample on Codility and finished the test task.

  The problem description is very short:

  The equilibrium index of a sequence is an index such that the sum of elements at lower indexes is equal to the sum of elements at higher indexes. For example, in a sequence A:

  $$A[0]=-7, A[1]=1, A[2]=5 ,A[3]=2, A[4]=-4, A[5]=3, A[6]=0$$

  \$3\$ is an equilibrium index, because:

  $$A[0]+A[1]+A[2]=A[4]+A[5]+A[6]$$

  \$6\$ is also an equilibrium index, because:

  $$A[0]+A[1]+A[2]+A[3]+A[4]+A[5]=0$$

  (The sum of zero elements is zero) \$7\$ is not an equilibrium index - because it is not a valid index of sequence A. If you still have doubts, here is a precise definition: The integer \$k\$ is an equilibrium index of a sequence \$A[0],A[1]\dots,A[n-1]\$ if and only if \$0\le k\$ and \$\sum(A[0\dots(k-1)])=\sum(A[(k+1)\dots(n-1)])\$. Assume the sum of zero elements is equal to zero.

The code I wrote gives the correct result, but, I only scored 8% on correctness and a 54% on performance. Here is my code:

// you can write to stdout for debugging purposes, e.g.
// console.log('this is a debug message');

function solution(A) {

    // write your code in JavaScript (Node.js 4.0.0
    var arrLen = A.length;

    var i = 0;

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

        //console.log("i: " + i);

         var firstSum = 0;
        var secSum = 0;

        var currIndex = i + 1;

        //get first chunk
        var j = 0;
        var lenJ = currIndex;

        for(j = 0; j < lenJ; j++){

            //console.log("firstChink: " + j);
            firstSum += A[j];

        }

        //get second chunk
        j = currIndex + 1;
        lenJ = arrLen;

        for(j; j < lenJ; j++){

            //console.log("secChunk: " + j);
            secSum += A[j];

        }

        if(firstSum == secSum){

            return currIndex;

        }

    }

    return arrLen;

}

Can somebody explain why my code scored so low? And what to do to improve?

I'm piggy backing on @jonah's answer. The first code snippet returns what Codility is asking for: any equilibrium index. Second code snippet returns all answers like in @jonah's suggestion. I also added two accumulating arrays for side by side comparison. You can add a breakpoint at the end of the function to compare the two arrays.

var A = [-1, 3, -4, 5, 1, -6, 2, 1];

function solution(A) {
    var rightSum = A.reduce((acc, val) => acc + val, 0), leftSum = 0;

    for (var i = 0; i < A.length; i++) {

        rightSum -= A[i];

        // Codility is asking for any, so return first found
        if (rightSum === leftSum) return i;

        leftSum += A[i];
    }

    return -1;
}

solution(A);

var A = [-1,3,-4,5,1,-6,2,1];

function solution(A) {

    var rightSum = A.reduce((acc, val) => acc + val, 0), leftSum = 0, rightSums = [], leftSums = [], answers = [];

    for (var i = 0; i < A.length; i++) {

        rightSum -= A[i];

        // You can keep track of answers for comparison/debugging
        rightSums.push(rightSum);
        leftSums.push(leftSum);

        if (rightSum === leftSum) answers.push(i);

        leftSum += A[i];

    }

    return answers;
}

solution(A);


Optimization for SQLite result set parsing I am retrieving information from an SQLite database that gives me back around 20 million rows that I need to process. This information is then transformed into a dict of lists which I need to use. I am trying to use generators wherever possible.

What optimizations can be done?

I am either getting a “Killed” message or it takes a really long time to run. The SQL result set part is working fine. I tested the generator code in the Python interpreter, and it doesn’t have any problems. I am guessing the problem is with the dict generation.

Update for clarity:

I have 20 million rows in my result set from my SQLite database. Each row is of the form:

(2786972, 486255.0, 4125992.0, 'AACAGA', '2005’)

I now need to create a dict that is keyed with the fourth element ‘AACAGA’ of the row. The value that the dict will hold is the third element, but it has to hold the values for all the occurences in the result set.

So, in our case here, ‘AACAGA’ will hold a list containing multiple values from the SQL result set. The problem here is to find tandem repeats in a genome sequence. A tandem repeat is a genome read (‘AACAGA’) that is repeated at least three times in succession.

For me to calculate this, I need all the values in the third index as a list keyed by the genome read, in our case ‘AACAGA’. Once I have the list, I can subtract successive values in the list to see if there are three consecutive matches to the length of the read. This is what I aim to accomplish with the dictionary and lists as values.

#!/usr/bin/python3.3
import sqlite3 as sql

sequence_dict = {}
tandem_repeat = {}

def dict_generator(large_dict):
     dkeys = large_dict.keys()
     for k in dkeys:
         yield(k, large_dict[k])

 def create_result_generator():
     conn = sql.connect('sequences_mt_test.sqlite', timeout=20)
     c = conn.cursor()
     try:
         conn.row_factory = sql.Row
         sql_string = "select * from sequence_info where kmer_length > 2"
         c.execute(sql_string)
     except sql.Error as error:
         print("Error retrieving information from the database : ", error.args[0])
     result_set = c.fetchall()
     if result_set:
         conn.close()
         return(row for row in result_set)

 def find_longest_tandem_repeat():
     sortList = []
     for entry in create_result_generator():
         sequence_dict.setdefault(entry[3], []).append(entry[2])

     for key,value in dict_generator(sequence_dict):
         sortList = sorted(value)
         for i in range (0, (len(sortList)-1)):
             if((sortList[i+1]-sortList[i]) == (sortList[i+2]-sortList[i+1])
                 == (sortList[i+3]-sortList[i+2]) == (len(key))):
                   tandem_repeat[key] = True
                   break

     print(max(k for k, v in tandem_repeat.items() if v))

 if __name__ == "__main__":
     find_longest_tandem_repeat()

I have missed the whole point of your program but these comments might be useful to you anyway :

dict_generator

I might be wrong but dict_generator(large_dict) looks like large_dict.iteritems() (Python 2) / large_dict.items() (Python 3).

create_result_generator

 while True:
     result_set = c.fetchall()
     if not result_set:
         break
     else:
         return(row for row in result_set)

This loop does not seem very loopy to me. What about :

 result_set = c.fetchall()
 if result_set:
     return(row for row in result_set)

Also, it seems to hilight a bigger issue about the fact that the connection might not be closed.

find_longest_tandem_repeat

In :

     try:
         sequence_dict[row[3]].append(row[2])
     except KeyError:
         sequence_dict[row[3]] = []
         sequence_dict[row[3]].append(row[2])

seems to be equivalent to

     sequence_dict.setdefault(row[3],[]).append(row[2])

Also, I don't know where row is coming from at this stage.

In :

 for key,value in dict_generator(sequence_dict):
     sortList = sorted(value)
     for i in range (0, (len(sortList)-1)):
         if((sorList[i+1]-sorList[i]) == (sorList[i+2]-sorList[i+1])
             == (sorList[i+3]-sorList[i+2]) == (len(key))):
               tandem_repeat[key] = True
     sortList = []

Naming (sortList/sorList) is either wrong or very confusing.
There is no point in doing sortList = [].
You can probably break once you've done tandem_repeat[key] = True because new iterations won't do much anyway.

     print(max(k for k in tandem_repeat.keys() if tandem_repeat[k] is True))

might as well be :

     print(max(k for k,v in tandem_repeat.items() if v))


Optimizing nested if statements in switch I have this code and I think it is very slow, about 50ms. I need to make it execute faster:

 switch (unit)
   {
       case 0:
       if (u == 2)
       {
           if (u1 == "piece" || u1 == "gram")
               result = quantity * y;
           else
               result = quantity * z;
       }
       if (u == 1)
       {
           if (u1 == "piece" || u1 == "gram")
               result = quantity * y;
           else
               result = quantity;
       }
       res_name = "gram";
       break;

       case 1: //ml
       if (u == 2)
       {
           if (u1 == "piece" || u1 == "gram")
               result = quantity * x;
           else
               result = quantity * y;

       }
       if (u == 0)
       {
           if (u1 == "piece" || u1 == "gram")
               result  = quantity * x;
           else
               result = quantity;
       }
       res_u = "ml";
       break;

case 2:
    res_u = "pieces";
    break;
 }

I agree with Romoku.  Unless there is a definite requirement to make this faster and a noticeable effect on performance of your application within this method I would consider making it more DRY, more developer friendly and less procedural.

Without more code and explanation of whats there here are some things I would consider:

Try and remove duplicated code.  I notice you repeat your if(u1 == "piece" ... code everywhere.  Consider moving that into a method and passing in the parameters required.

private int GetConversion(UnitsEnum units, int pieceGramsMultipler, int otherMultipler)
{
  return units == UnitsEnum.Piece || units == UnitsEnum.Gram
                 ? pieceGramsMultipler
                 : otherMultipler;
}

Then used like

if (u == 2)
{
   result = quantity * GetConversion(u1, y, z);
}

Consider changing your string literals into enumerations. So "piece" and "gram" might become:

private enum UnitsEnum
{
  Piece,
  Gram
}

Consider changing your case statements into constants or enumerations.  So rather than case 1, it might be case Metric.Ml or ML or Milliliters etc
Offer better names for your variables.  u1, x, y, z gives me no idea of what they are trying to do.
Use if else instead of 2 if statements.  If u == 2 then there is no chance that it will be == 1 so convert your if's into if elses to make that more obvious.

Otherwise, perhaps posting the full code that this piece resides in might get your more help and suggestions.  I get the feeling that there is more help this community can offer you here than just what you have posted.