c# best way to declare a class-level value in base class and set it at derived classes I have a root class and I'd like to add an indicator to it (property, static field, readonly field, virtual method or something like that), that should give me the same string value for every instance of that class, I would rather prefer to get that value without instantiating that class. GetObjType() virtual method is my current implementation of it but I am thinking there may a better way of doing this. Can you think of a better way of doing this, like class level singleton fields or something like that.

public abstract class CoreEntity
{
    public int Id { get; set; }
    public abstract string GetObjType();
}
public class DepartmanBase : CoreEntity
{
    public override string GetObjType()
    {return "DEPARTMAN";}
    public string DepartmanName { get; set; }
    public int ManagerId { get; set; }
    //Obj Type should be set as "DEPARTMAN" here and should be same in both derived classes
}
public class Departman : DepartmanBase
{
}
public class DepartmanView : DepartmanBase
{
    public string ManagerName { get; set; }
}

public class Personnel : CoreEntity
{
    public override string GetObjType() {return "PERSONEL";}
    public string PersonnelName { get; set; }
}

public class Repository<T>
    where T : CoreEntity , new()
{
    public void DoSomething()
    {
        string objType = new T().GetObjType();
        Console.WriteLine("Object Type for: " + typeof(T).Name  + " is " + objType);
    }
}

void Main()
{
    var d = new Repository<Departman>();
    var dv = new Repository<DepartmanView>();
    var p = new Repository<Personnel>();

    d.DoSomething();
    dv.DoSomething();
    p.DoSomething();
}

What I've guessed from your code (and I might be wrong) is that you need to get some string from a type used in a persistence layer. The string might be a table name, or a value for a certain column. I've decided to call this a discriminator. Multiple types can have the same discriminator but each type can only have one.

I think it makes sense to introduce an inheritable attribute here, let's do that:

[AttributeUsage(AttributeTargets.Class, Inherited = true, AllowMultiple = false)]
sealed class DiscriminatorAttribute : Attribute
{
    readonly string discriminator;

    public DiscriminatorAttribute(string discriminator)
   {
        this.discriminator = discriminator;
   }

    public string Discriminator
    {
        get { return discriminator; }
    }
}

We'll also add a class which takes a type and gets us our string (if we have one).

public static class DiscriminatorService
{
    public static string LookupDiscriminatorForType(Type t)
    {
        var result = (from a in t.GetCustomAttributes(true)
                 where a is DiscriminatorAttribute
                 select a).SingleOrDefault() as DiscriminatorAttribute;
        return result != null ? result.Discriminator : null;
    }
}

This will allow you to change your repository to the following:

public class Repository<T> where T : CoreEntity
{
    public void DoSomething()
    {
        var objType = DiscriminatorService.LookupDiscriminatorForType(typeof(T));
        Console.WriteLine("Object Type for: " + typeof(T).Name  + " is " + objType);
    }
}

Some important things to note:

You no longer need to create an instance to get the string
you can remove the new() generic constraint

Obviously you need to decorate your classes with the new attribute:

[Discriminator("DEPARTMAN")]
public class DepartmanBase : CoreEntity
{
    public string DepartmanName { get; set; }
    public int ManagerId { get; set; }
}
public class Departman : DepartmanBase
{
    // Inherits discriminator from DepartmanBase
}

public class DepartmanView : DepartmanBase
{
    // Inherits discriminator from DepartmanBase
    public string ManagerName { get; set; }
}

[Discriminator("PERSONEL")]
public class Personnel : CoreEntity
{
    public string PersonnelName { get; set; }
}

Here's the full code in .Net Fiddle


Python Binary Search Performance This is my version of the binary search method. What more can I do to optimize this?

I have written comments throughout to guide anyone who might find this code helpful through how it operates. But I would like to update it if it can be made faster/cleaner.

def bSearch(arr, val, first, last):

    #Find the index of the middle array value.
    mid = (len(arr)-1)//2

    #If there are no more numbers to search through
    if first >= last:

        #Check if the last number is the number being searched for.
        if (arr[mid] == val):
            print("TRUE")
        else:
            print("FALSE")

    #If there are still numbers to search through
    #check array at index (mid) for the desired value.
    elif (arr[mid] == val):
        print("TRUE")

    elif (arr[mid] < val):
        #nArr = new array = greater half of the original array.
        nArr = arr[len(arr)//2:]
        bSearch(nArr, val, 0, len(nArr)-1)

    elif (arr[mid] > val):
        #nArr = new array = lesser half of the original array.
        nArr = arr[:len(arr)//2]
        bSearch(nArr, val, 0, len(nArr)-1)

#The array used for testing.
a = [1,2,3,4,6,7,8,10]

print("Test One")
#Search for 5 in array a. Should return false.
bSearch(a, 5, 0, len(a)-1)

print("Test Two")
#Search for 2 in array a. Should return true.
bSearch(a, 2, 0, len(a)-1)

print("Test Three")
#Search for 8 in array a. Should return true.
bSearch(a, 8, 0, len(a)-1)

print("Test Four")
#Search for 1 in array a. Should return true.
bSearch(a, 1, 0, len(a)-1)

print("Test Five")
#Search for 15 in array a. Should return false.
bSearch(a, 15, 0, len(a)-1)

Non-recursive approach:

# Binary search in a non-empty sorted array
def bin_search(a, val):
    l = 0
    r = len(a)
    # Trying to find val in a[l:r]
    while r - l > 1:
        m = (l + r) // 2
        if a[m] > val:
            r = m
        else:
            l = m
    return a[l] == val


Haskell program to extract the first words of each line of a text file isLineTerminator c = c == '\r' || c == '\n'
leadWords [] = []
leadWords cs =
    let (pre, suf) = break isLineTerminator cs
    in  (head (words pre)) : case suf of
                            ('\r':'\n':rest) -> leadWords rest
                            ('\r':rest)      -> leadWords rest
                            ('\n':rest)      -> leadWords rest
                            _                -> []

fixLines :: String -> String
fixLines input = unlines (leadWords input)

interactWith function inputFile outputFile = do
    input <- readFile inputFile
    writeFile outputFile (function input)

main = mainWidth myFunction
  where mainWidth function = do
            args <- getArgs
            case args of
                [input, output] -> interactWith function input output
                _               -> putStrLn "error: exactly two arguments needed"
        myFunction = fixLines

Much of this program is written by the author of the book on Haskell that I'm reading, so I assume that much of it is good. However, my part is in altering the leadWords function. Does it look like a:

readable, elegant function?
efficient and optimized function?

Errors

The code block above is an example where your program will fail. Any empty line will lead to an empty list in words "" and therefore to head [], which calls error.

Standard library

That being said, there are some other issues with your code. You use unlines in fixLines, yet leadWords is essentially lines, although it also uses head . words internally. If we rewrite leadWords with map, we end up with

leadWords = map (head . words) . lines

which immediately shows the possible error on empty lines. Still, we can fix this if we write another function:

firstWord :: String -> Maybe String
firstWord xs = case words xs of
                   (x : _) -> Just x
                   _       -> Nothing

Now we can use mapMaybe from Data.Maybe to write leadWords:

leadWords :: String -> [String]
leadWords = mapMaybe firstWord . lines

Type signatures

Other than that, you should really add type signatures to your top-level functions. Only fixLines has one, and its not clear why that one got special handling.

Code complexity

main is overly complicated, too. We can just inline mainWidth, as its local binding doesn't yield any more comfort:

main :: IO ()
main =  do
    args <- getArgs
    case args of
        [input, output] -> interactWith fixLines input output
        _               -> putStrLn "error: exactly two arguments needed"

Alternatively, we can provide mainWith as top level function and use that in main.

Complete code

If we apply all remarks given above, we end up with something similar to the following code:

import Data.Maybe (mapMaybe)

mainWith :: (String -> String) -> IO ()
mainWith f = do
    args <- getArgs
    case args of
        [input, output] -> interactWith f input output
        _               -> putStrLn "error: exactly two arguments needed"

fixLines :: String -> String
fixLines = unlines . leadWords

main :: IO ()
main = mainWith fixLines

-- for completeness

leadWords :: String -> [String]
leadWords = mapMaybe firstWord . lines

firstWord :: String -> Maybe String
firstWord xs = case words xs of
                     (x : _) -> Just x
                     _       -> Nothing

-- That's just personal preference, your style is fine.
interactWith :: (String -> String) -> FilePath -> FilePath -> IO ()
interactWith f input output = readFile input >>= writeFile output . f

In short:

add type signatures to top-level functions
don't use head on lists that might be empty
use standard library types when possible (Maybe)
use standard library functions when possible (map, lines and mapMaybe)


Assigning an array of different (but convertible) type to concurrency::array in C++AMP I am creating a vector class which can be manipulated on the GPU and I am using C++AMP for the GPU accelerated code.

I am wondering the most efficient way of assigning elements of a different type to a concurrency::array in C++AMP. There are two ways that I can think of (but there may be more):

Firstly, one could use a simple sequential for loop and use the concurrency::array_view object to assign the data members and then call concurrency::array_view::synchronize at the end to ensure the data is stored on the accelerator for subsequent operations, this would look like following:

template <typename T>
CVector& operator=( T tArray[Size] )
{
    static_assert( std::is_convertible<T, NumType>::value, "Cannot assign a vector from an array of non-convertible type" );

    concurrency::array_view<NumType, 1> avThis( m_numArray.view_as( m_numArray.extent ) );

    for( std::size_t s = 0; s < Size; ++s )
    {
         avThis(s) = static_cast<NumType>(tArray[s]);
    }

    avThis.synchronize();
    return *this;
}

Alternatively, one could construct a concurrency::array_view from the array and then use a concurrency::parallel_for_each to assign the data members as follows:

template <typename T>
CVector& operator=( T tArray[Size] )
{
    static_assert( std::is_convertible<T, NumType>::value, "Cannot assign a vector from an array of non-convertible type" );

    concurrency::array_view<const T, 1> avArray( Size, tArray );

    concurrency::parallel_for_each( m_numArray.extent, [=, &m_numArray]( concurrency::index<1> index ) restrict(amp) {
         m_numArray[index] = static_cast<NumType>(avArray[index]);
    } );

    return *this;
}

Which of these methods results in better performance or are they equal?

I profiled the code, and using the Microsoft Concurrency Visualizer, produced the following graph:

As with almost any performance related question, the correct answer is probably to do some profiling to get solid answers.

That said, it strikes me as unlikely that attempting to do the copy in parallel will usually make much difference. In a typical case, copying to the accelerator takes place over a PCIe bus. In that typical case, even a single CPU core will have substantially higher memory bandwidth than that bus, so the copy will be a "hurry up and wait" situation; using more than one CPU will basically means hurrying even faster so you can wait even longer.

In the case of something like an AMD APU, the parallel copy stands at least a little more chance of improving performance. Even here I'd have substantial doubts though. In particular, a single core can still normally (more than) saturate the processor's bandwidth to main memory, so doing the copy in parallel still won't gain anything.

That leads us to a much more general conclusion: if your processing is memory bound to start with, parallel execution is unlikely to improve performance unless you do it on hardware that has multiple sockets and each socket has a separate memory controller. In this case, the code will probably still be limited by bandwidth to memory, but you've doubled the bandwidth to memory which can roughly double the speed of the code.


Best way to call future.get() The following is a small part of some code I am working on
Which is better/faster/nicer?

1.

for (Future<Subscription> future : futures) {
      try {
        executorService.submit(new Updater(future.get()));
      } catch (ExecutionException e) {
        logger.error(e, e);
      }
    }

2.

  for (Future<Subscription> future : futures) {
          try {
            executorService.submit(new Updater(future));
          //here future.get() will be called in the Updater constructor in its own thread
          } catch (ExecutionException e) {
            logger.error(e, e);
          }
        }

I am tempted to go with the second way as future.get() is called in its own thread and not in the main thread.

Your presumtion that the future.get() method is called in another thread is wrong. The  constructor still executes in the same thread as the loop, its the run() method in the Updater class that is called in its own thread. Consider passing the whole future to the Updater and store a reference to the future internally. Then, once the value is really required (somewhere after the run method of Updater has been called), you can call get() on the future to retrieve its value.


PHP loop to generate an HTML form for the days in the coming week There are two arrays, $states and $weekdays. I want to loop through them to create a form, an HTML table, or possibly some other element).

Here's my example:

// addDay() Adds 1 day to the date
// format() Returns a date in a new format
// get_date() returns todays date in unix

$weekdays = [ 'Monday', 'Tuesday', 'Wednesday', 'Thursday', 'Friday', 'Saturday', 'Sunday' ];
$states   = [ 'start', 'finish', 'total', 'overtime' ];
$date     = get_date();

foreach ( $states as $state ) { ?>
  <tr class="<?php echo $state; ?>">
    <?php echo $state; ?>
    <?php
    $d = 0;
    foreach ( $weekdays as $weekday ) { ?>

        <input name="date<?php echo $date->addDay( $d )->format('[Y][m][d]') . '[' . $state . ']'; ?>" />

    <?php
    $d++;
      } ?>

<?php
}

I think the code can be cleaner. Ignore the fact that there aren't comments. Focus on the HTML / PHP intertwining. Can it be separated further or tidier?

Is there a method of sprintf but for sections of code perhaps? or would you echo the whole thing with double quotes for the variables?

Note — I am using PHP7, so be as fancy as you want with the latest syntax;

Initial Thoughts

Initially I was thinking of suggesting a view class that could hold the states and weekdays in instance variables, then have methods to get the header for each state and then display the cells for each weekday, but that seems like a lot of extra overhead just to display a table, and wouldn't provide the desired separation of business logic and display templates.

Simple improvements

As GrumpyCrouton mention in the SO answer, PHP Short echo tags could be used to shorten the inline echo statements (i.e. <?php echo to <?=).

$d could be eliminated by using the associative array syntax of foreach

  foreach (array_expression as $key => $value)
               statement
  1

foreach($weekdays as $d => $weekday) {
    //$d doesn't need to be incremented since it is the keys - e.g. 0, 1, 2, etc.
}

Or just used array_keys() since the weekday value isn't used...

foreach(array_keys($weekdays) as $d) {
    //$d doesn't need to be incremented since it is the keys - e.g. 0, 1, 2, etc.
}

One could define a template using Heredoc syntax, like:

$cellTemplate = <<<CELL

    <input name="date-%s" />

CELL;

Then that could be used for each table cell:

foreach ( $states as $state ) { ?>
  <tr class="<?= $state; ?>">
    <?= $state; ?>
    <?php
    foreach ( $weekdays as $d => $weekday ) {
        $dateName = $date->addDay( $d )->format('[Y][m][d]') . '[' . $state . ']';
        echo sprintf($cellTemplate, $dateName);
      }
      ?>

<?php
}
?>

A demonstration can be seen here. Note that instead of using the Carbon DateTime I am using PHP's DateTime class along with DateInterval to accomplish the date addition with DateTime::add().

Template Engines

One could also utilize a template engine, like Smarty, Symfony’s Twig, Laravel’s Blade, etc.

Here is an example with a Smarty template. I like bumperbox's notion of storing the formatted weekdays in an array and using that in later on. That array could also be generated using array_map() (though accessing $date inside the callback would require a use statement):

$formattedWeekdays = array_map(function($d, $weekday) use ($date) {
    return $date->addDay( $d )->format('[Y][m][d]');
}, array_keys($weekdays), $weekdays);
$smarty->assign('states', $states);
$smarty->assign('formattedWeekdays', $formattedWeekdays);
$smarty->display('../templates/weekdays.tpl');

And that template file (i.e. weekdays.tpl) might look like this:

    {foreach $states as $state}

        {$state}
        {foreach $formattedWeekdays as $formattedWeekday }

            <input name="date-{$formattedWeekday}[{$state}]" />

        {/foreach}

    {/foreach}

That way the display logic is separate from the business logic. As this wikibooks page alludes to, the HTML is separated from the PHP code so multiple people (e.g. graphics designer, developers, etc.) can manage the separate parts since they are de-coupled.

1http://php.net/manual/en/control-structures.foreach.php


Return office details from multiple tables The function itself is just returning as void. But, the point of the posted code is not about what the function is returning. It is the code related to using T-SQL and C# to return data from a SQL database that I would like reviewed. Especially the way the using statements are structured.

    public void GetOffice(int syncID)
    {
        string strQry = @"
Select so.SyncID,
   so.title
From Offices o
Left Outer Join SyncOffices so On so.id = o.SyncID
Where o.SyncID = @syncID
";

        using (SqlConnection conn = new SqlConnection(Settings.ConnectionString))
        {
            using (SqlCommand objCommand = new SqlCommand(strQry, conn))
            {
                objCommand.CommandType = CommandType.Text;
                objCommand.Parameters.AddWithValue("@syncID", syncID);
                conn.Open();
                SqlDataReader rdr = objCommand.ExecuteReader();

                if (rdr.Read())
                {
                    this.OfficeName= rdr.GetString(1);
                }

                rdr.Close();
            }
        }
    }

First of all, kudos for using a Parameter and not concatenating the value into your T-SQL string.

You're not disposing all IDisposable objects. SqlDataReader should be disposed as well. Now this makes it quite a bunch of nested using scopes, which you could rework like this:

    using (var connection = new SqlConnection(Settings.ConnectionString))
    using (var command = new SqlCommand(sql, connection))
    {
        command.CommandType = CommandType.Text;
        command.Parameters.AddWithValue("@syncID", syncId);
        connection.Open();

        using (var reader = command.ExecuteReader())
        {
            if (reader.Read())
            {
                this.OfficeName = reader.GetString(1);
            }
        }
    }

Note:

Usage of var for implicit typing makes the code easier to read (IMO), if you're using C# 3.0+
Disemvoweling is bad. There's no reason to call a variable rdr over reader. Use meaningful names, always.
Hungarian notation is evil. There's no reason to prefix a string with str.
Stick to camelCasing for locals - that includes parameters, so syncID becomes syncId.

The code assumes the query only returns 1 row, but the query isn't written to explicitly select a single row. This could lead to unexpected results.

Given some IList<string> results = new List<string>();:

        using (var reader = command.ExecuteReader())
        {
            while (reader.Read())
            {
                results.Add(reader.GetString(1));
            }
        }

You could then do this.OfficeName = results.Single(); (which would blow up if no rows were returned). One thing that strikes me, is that you're selecting 2 fields, but only using 1, which makes this reader.GetString(1) statement look surprising. If you don't need to select the SyncID field, remove it from your query and do reader.GetString(0) instead.

Finally, the T-SQL itself:

Select so.SyncID, so.title
From Offices o
Left Outer Join SyncOffices so On so.id = o.SyncID
Where o.SyncID = @syncID

Could look like this:

SELECT so.Title
FROM Offices o
LEFT JOIN SyncOffices so ON so.id = o.SyncID
WHERE o.SyncID = @syncID

Or, in a string:

var sql = "SELECT so.Title FROM Offices o LEFT JOIN SyncOffices so ON so.Id = o.SyncId WHERE o.SyncId = @syncId";

The line breaks make it look weird, and since it's not too long of a query, I think it would make the code better to have it on a single line.


Simple neural network implementation in Python A simple neural network I wrote in Python without libraries. I avoided implementing it in matrix form because I sought to get a basic understanding of the way NN's work first. For that reason I'm strongly favoring legibility over efficiency. I tried to keep my code readable and pyhonic, any style feedback would be particularly appreciated.

A quirk about this design is it does back propagation on a per training example basis and uses momentum to try and avoid over fitting to specific examples.
Also I realized I never added base values to the neurons, it seems to work alright with out them but if anyone has a more in depth understanding of why you'd want them I'd be curious to hear about that.

import math
import random

import data

def sigmoid(x):
    return 1 / (1 + math.exp(-x))

def sigmoid_prime(x):
    return x * (1.0 - x)

def loss(x,y):
    return sum([(a-b)**2 for (a,b) in zip(x,y)])

class Neuron():
    learning_rate = 0.015
    momentum_loss = 0.03

    def __init__(self, input_neurons):
        self.weights = [random.uniform(-1,1) for _ in range(input_neurons)]
        self.momentum = [0 for _ in range(input_neurons)]

    def forward(self, inputs):
        dot = sum([x*y for (x,y) in zip(inputs, self.weights)])
        self.output = sigmoid(dot)
        return self.output

    def backpropagate(self, inputs, error):
        error_values = list()
        gradient = error * sigmoid_prime(self.output)
        for i, inp in enumerate(inputs):
            self.nudge_weight(i, gradient * inp)
            error_values.append(self.weights[i]  * gradient)
        return error_values

    def nudge_weight(self, weight, amount):
        change = amount * Neuron.learning_rate
        self.momentum[weight] += change
        self.momentum[weight] *= (1 - Neuron.momentum_loss)
        self.weights[weight] += change + self.momentum[weight]

class Network():
    def __init__(self, topology):
        self.layers = list()
        for i in range(1,len(topology)):
            self.layers.append([Neuron(topology[i-1]) for _ in range(topology[i])])

    def forward(self, data):
        output = data
        for layer in self.layers:
            output = [neuron.forward(output) for neuron in layer]
        return output

    def backpropagate(self, data, output, target):
        error_values = [tval - output for (tval, output) in zip(target, output)]
        for i in range(len(self.layers)-1,0,-1):
            layer_output = [neuron.output for neuron in self.layers[i-1]]
            error_values = self.backpropagate_layer(i, error_values, layer_output)
        self.backpropagate_layer(0, error_values, data)

    def backpropagate_layer(self, layer, error_values, inputs):
        next_errors = list()
        for neuron, error in zip(self.layers[layer], error_values):
            bp_error = neuron.backpropagate(inputs,error)
            if not next_errors:
                next_errors = bp_error
            else:
                next_errors = [a+b for a,b in zip(next_errors,bp_error)]
        return next_errors

The full source code for project including the data base and some other testing code can be found here: https://github.com/RowanL3/Neural-Network

A more pythonic way of writing self.momentum = [0 for _ in range(input_neurons)] would be self.momentum = [0]*input_neurons


Schema.org Microdata validation plugin I am starting to write a plugin that is aiming to validate a piece of mark up against requirements of schema.org.

I was hoping if I could get some more tips on how to improve the structure of my code. Is this how you would go about this if you were to do it?

    $.fn.hasAttr = function(name) {
       return this.attr(name) !== undefined;
    };

    $.fn.outerHTML = function(s) {
        return $(this).clone().wrap('
').parent().html();
    };

    $.fn.getOpeningTag = function (s) {
      return $(this).outerHTML().slice(0, $(this).outerHTML().indexOf(">") + 1)
    }

    $.fn.validateSchema = function(options) {

      var defaults = {
        // for a list of international postcode regex see: http://www.thalesjacobi.com/Regex_to_validate_postcodes
        // The one used in the example is from the UK
        localPostCodeFormat: new RegExp("^([Gg][Ii][Rr] 0[Aa]{2})|((([A-Za-z][0-9]{1,2})|(([A-Za-z][A-Ha-hJ-Yj-y][0-9]{1,2})|(([A-Za-z][0-9][A-Za-z])|([A-Za-z][A-Ha-hJ-Yj-y][0-9]?[A-Za-z])))) {0,1}[0-9][A-Za-z]{2})$"),
        utilities : {
            missAtrr : function (target, attrName) {
              if (!target.hasAttr(attrName)){
                 defaults.returnError(target, "The element " + target.getOpeningTag()  + " is missing the required attribute " + attrName);
              }
            },
            isValidPostCode : function (target) {
              if (!defaults.localPostCodeFormat.test(target.text())) {
                defaults.returnError(target, target.getOpeningTag() + " does not have a valid post code");
              }

            }
        },
        returnError : function (target, errorMessage) {
          var errorContainer = $('');
          target.before(errorContainer);
          errorContainer.css({
            'color' : 'red',
            'border': 'solid 1px red'
            }).text(errorMessage);
        } // more utils to be added
      };
      // Extend our default options with those provided.
      var opts = $.extend(defaults, options);

      var schemaElements = {
            postalAddress : $(this).find('[itemtype*=PostalAddress]'),
            itemProps : $(this).find("[itemprop]"),
            emptyProps : $(this).find("[itemprop='']"),
            postCode : $(this).find("[itemprop=postalCode]"),
            email : $(this).find("[itemprop=email]"),
            outofplaceChild : $(this).find("[itemscope]").siblings("[itemprop]")// more dom refs to be added
        }

      // validation stuff
      //
      // PostalAddress should have itemsscope
      if (schemaElements.postalAddress){
        defaults.utilities.missAtrr(schemaElements.postalAddress, "itemscope");
      }
      // No itemscope can be left empty
      if (schemaElements.emptyProps){
        $.each(schemaElements.emptyProps, function () {
            defaults.returnError($(this), $(this).getOpeningTag() + " can not be left without a value");
        });
      }
      // postcode should match the post code format of that country
      if (schemaElements.postCode){
        defaults.utilities.isValidPostCode(schemaElements.postCode);
      }
      // No itemprop can exist without having a itemscope parent
      if (schemaElements.outofplaceChild){
        $.each(schemaElements.outofplaceChild, function () {
            defaults.returnError($(this), $(this).getOpeningTag() + " Does not have a itemscope parent");
        });
      }
      // more rules to be added
    };

    $(function(){
        $('body').validateSchema();
    });

This will return false for attr in such an object {'attr': undefined}:

$.fn.hasAttr = function(name) {
   return this.attr(name) !== undefined;
};

If this is not exactly what you want, have a look at related posts (one, two) and search for much more.

This (and similars) will always evaluate to true, if schemaElements object has the postalAddress key:

if (schemaElements.postalAddress){
    defaults.utilities.missAtrr(schemaElements.postalAddress, "itemscope");
}

here's an example session from Chrome developer console:

var rules = {'a': $('html').find('#idontexist')};
rules.a;
// []
if (rules.a) {console.log(true)} else {console.log(false)};
// true

Be consistent with ; at the end of the lines.


Create HTML page to document contents of directory I often work on embedded system projects in which I have a number of documentation files, such as microprocessor datasheets, schematics, bills of materials, etc.  I find it convenient to create a simple little HTML file that contains the file descriptions and a hyperlink to each file.  I used to create these all by hand, but I now have an automated script that does that for me.  I still have to edit the file to include the descriptions of the files, but this creates both an index.html and style.css file as a start.  I'd be interested in a review of both the Python code and the HTML and Javascript and CSS that it creates.  The code is intended solely for use on a Linux box.

Invocation and use

This is designed to placed in the path and invoked from the command line from within the directory to be documented.  Then the resulting HTML file is invoked in the same way as with firefox index.html.  Although it doesn't make much difference in this case, the intent is that the HTML file is not served from a web server but simply interpreted from the file system so that the entire project, with documentation, can be put on a CD or DVD (or other read-only media) and used without having to run a web server.

docdir.py

#! /usr/bin/python3
from os import walk

style = '''\
body
{
    font-style:normal;
    font-family:sans-serif,Georgia, Helvetica, Arial, sans-serif;
        background-color:white;
    color:black;
    margin-left:5%;
}
div.navbar
{
    text-align:center;
}
h1
{
    text-indent:1em;
    color:white;
    background-color:navy;
}
h2
{
    text-indent:2em;
    color:white;
    background-color:navy;
}
h3
{
    margin-left:1em;
    margin-right:1em;
    text-indent:2em;
    color:white;
    background-color:navy;
}
div.trailer table
{
    width:100%;
    border-style:none;
    padding-left:0;
}
table
{
    border-style:solid;
}
td
{
    padding-left:20px;
}
caption
{
    font-style:oblique;
}'''

index_tail='''\
];

function loadLinks() {
    var table = document.getElementById("docTable");
    links.reverse();
    links.forEach(function(element, index, array) {
        var row = table.insertRow(0);
        var cell1 = row.insertCell(0);
        var cell2 = row.insertCell(1);
        cell1.innerHTML = "" + element.name + "";
        cell2.innerHTML = element.desc;
    });
}
</script>

Documents

These directories may contain proprietary and confidential information.

Document list

&nbsp;

    index.html

this file

'''

if __name__=="__main__":
    title="Documents"
    index_head=f'''\
<!DOCTYPE html>

<title>{title}</title>
<link rel="stylesheet" href="style.css">

(Warning I havn't tested the following code.)

Python

Offload the custom html to a template engine. Jinja2 can be used in both flask and django. And so I'll change your code to use this. (I'm not affiliated)
It's good to see you manually .closeing your files. But using with is better.
Use ' or " string delimiter. You can switch if the string contains your prefered one. I prefer ', but would use "it's" rather than 'it\'s'.
You can move your CSS into its own file and copy the file with whatever you think is best.
I'd use ''.join and a comprehension to make filenames a string, that can be passed to Jinja.

#! /usr/bin/python3
from os import walk
from pathlib import Path
import shutil

from jinja import Environment, FileSystemLoader, select_autoescape

FILE = Path(__file__).resolve()
RESOURCES = FILE / 'name'
CWD = Path.cwd()

env = Environment(
    loader=FileSystemLoader(str(RESOURCES / 'templates')),
    autoescape=select_autoescape(['html'])
)

if __name__ == '__main__':
    shutil.copyfileobj(
        open(RESOURCES / 'css' / 'style.css'),
        open(CWD / 'style.css', 'w+')
    )

    filenames = ''.join(
        '{{ "name":"{0}", "desc":"{0}" }},'.format(f.name)
        for f in CWD.iterdir()
        if f.is_file()
    )

    with open(CWD / 'index.html', 'w+') as html:
        page = env.get_template('index.html').render(
            title='Documents',
            links=filenames
        )
        print(page, file=html)

CSS

From the little I've done with CSS your file looks a bit off to me:

I'm used to seeing a line between each style.
I'm used to seeing K&R, rather than Allman, indentation style.
You should keep your indentation consistant.
I'm used to seing a space after : and ,.

New location: /bin/name/css/style.css.

body {
    font-style: normal;
    font-family: sans-serif, Georgia, Helvetica, Arial, sans-serif;
    background-color: white;
    color: black;
    margin-left: 5%;
}

div.navbar {
    text-align:center;
}

HTML

I'm used to seing indentation, most webdev tools also do this for you. And so I'd assume that if they're doing it then there's a good reason for it. I find your HTML code to be a bit hard to read currently because of it.

New location: /bin/name/templates/index.html.

<!DOCTYPE html>

  <title>{{ title }}</title>
  <link rel="stylesheet" href="style.css">

Documents

    These directories may contain proprietary and confidential information.

Document list

&nbsp;

        index.html

        this file


LeetCode:implement strstr C# https://leetcode.com/problems/implement-strstr/

I implemented Rabin-Karp algorithm
https://en.wikipedia.org/wiki/Rabin%E2%80%93Karp_algorithm

please review for performance, also if you were in an interview what do you think about using functions like GetHashCode()?
would you like the interviewee to implement the hashing on their own?

  Implement strStr().

  Return the index of the first occurrence of needle in haystack, or -1
  if needle is not part of haystack.

Example 1:

Input: haystack = "hello", needle = "ll"
Output: 2
Example 2:

Input: haystack = "aaaaa", needle = "bba"
Output: -1
Clarification:

  What should we return when needle is an empty string? This is a great
  question to ask during an interview.

  For the purpose of this problem, we will return 0 when needle is an
  empty string. This is consistent to C's strstr() and Java's indexOf().

using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace StringQuestions
{
    /// <summary>
    ///https://leetcode.com/problems/implement-strstr/
    /// </summary>
    [TestClass]
    public class StrStrLeetCode
    {
        [TestMethod]
        public void ValidNeedleTest()
        {
            string haystack = "hello";
            string needle = "ll";
            Assert.AreEqual(2, StrStr(haystack, needle));
        }

        [TestMethod]
        public void NotValidNeedleTest()
        {
            string haystack = "aaaaa";
            string needle = "ba";
            Assert.AreEqual(-1, StrStr(haystack, needle));
        }

        public int StrStr(string haystack, string needle)
        {
            if (string.IsNullOrEmpty(needle))
            {
                return 0;
            }
            int n = haystack.Length;
            int m = needle.Length;
            var hash = needle.GetHashCode();
            for (int i = 0; i < n - m + 1; i++)
            {
                string tempStr = haystack.Substring(i, m);
                var hashTemp = tempStr.GetHashCode();
                if (hash == hashTemp)
                {
                    return i;
                }
            }

            return -1;
        }
    }
}

As dfhwze and Roland already pointed out, a hash alone is not sufficient to determine whether two things are equal, so you still need to do a string comparison afterwards if the hashes match. Otherwise you will get wrong results from time to time. Not to mention the effect of hash randomization between different application runs...

The idea behind Rabin-Karp's use of hashes is to replace costly string comparisons with cheap hash comparisons. But in your case, the cost of creating a substring and calculating its hash (which involves some calculations for every character) is often greater than doing a direct string comparison (which can bail out at the first difference).

As the Wikipedia article that you linked to says, you'll want to use a rolling hash, a hashing algorithm that allows you to calculate the hash of the next substring with just a few operations, regardless of how long that substring is.

Also, as far as I can tell, storing string.Length in a local variable doesn't offer any performance improvements. It does make the code slightly less readable though, in my opinion.


Find all the factors of a given natural number, N I was asked this question in an interview:

Find all the factors of a given natural number, N
1 <= N <= 10**10
Example:
N = 6 => 1, 2, 3, 6
N = 60 => 1, 2, 3, 4, 5, 6, 10, 12, 15, 20, 30, 60

I wrote following code,but got an feedback that complexity could be improved.

How to optimise following code?

import math
def get_all_factors(n):
    ans = [1]
    if n==1:
        return ans
    factors = [False] * n
    for i in xrange(2,int(math.sqrt(n))):
        if not factors[i]:
            if n%i == 0:
                factors[i] = True
                factors[n/i] = True

    for i, is_factor in enumerate(factors[1:], 1):
        if is_factor:
            ans.append(i)
    ans.append(n)
    return ans

ans = get_all_factors(60)
print [x for x in ans]

You don't need to keep an intermediate list here, just add all the divisors to a set:

from math import sqrt

def get_all_factors(n):
    divisors = set()
    for i in xrange(1, int(sqrt(n)) + 1):
        if n % i == 0:
            divisors.add(i)
            divisors.add(n / i)
    return divisors

if __name__ == "__main__":
    print get_all_factors(60)

I also added a if __name__ == "__main__": guard to allow importing this function from another script.

Note that there is no requirement for the output to be a sorted list in the problem statement you posted. If there is, you can just call sorted on the output of this.

As @kyrill mentioned in the comments, you could also make this a generator:

from math import sqrt, ceil

def get_all_factors(n):
    sqrt_n = sqrt(n)
    for i in xrange(1, int(ceil(sqrt_n))):
        if n % i == 0:
            yield i
            yield n / i
    if sqrt_n % 1 == 0:
        yield int(sqrt_n)


Bracket Pair Validation Validate that the following bracket pairs [], {}, () open and close successfully in a candidate String.

Valid String Example: "[{()}]" should be evaluated as true because no "outer" open bracket is closed before an "inner" open bracket.

Invalid String Example: "[f{o]o}"

I would appreciate feedback on my implementation:

Is there a case I'm missing?
Is there a cleaner way to validate bracket pairs?
Are there any particular performance concerns I'm ignoring?
I have hard-coded my bracket values. What if I want to add, say, ten more bracket values...is there a cleaner way to implement the open/close relationship? Use a Map, perhaps?

public boolean validateBracketPairs(final String string) {
        final Stack<Character> bracketStack = new Stack<>();
        for (final char character : string.toCharArray()) {
            if ('{' == character || '[' == character || '(' == character) {
                bracketStack.push(character);
            }

            else if (!bracketStack.empty() &&
                    (('{' == bracketStack.peek() && '}' == character)
                        || ('[' == bracketStack.peek() && ']' == character)
                        || ('(' == bracketStack.peek() && ')' == character)))  {
                bracketStack.pop();
            }

            else if ('}' == character || ']' == character || ')' == character) {
                return false;
            }
        }

        return bracketStack.empty();
    }

Right, so I've got a few suggestions.

First, instead of making ourselves worry about non-bracket characters, let's trim them out:

string = string.replaceAll("[^\\[\\]\\{\\}\\(\\)]", "");

You'll have to remove the final modifier, which I can't see a reason for anyway. Also, you never actually caused an error if character wasn't a bracket, so the code still ends up working the same. It's just less to worry about.

Note: Technically, since this code ignores all non-bracket characters, this statement isn't necessary. If you comment it out, the rest of the code should still work fine. Still, I like explicitly saying, in code, that my thing ignores your petty brackets and mustaches.

Second, let's define a private static final Map<Character, Character> MATCHING_CHARS1 to store the pairs, instead of hardcoding them. That way, instead of checking if something matches one of three hard-coded brackets, we can do this:

if (MATCHING_CHARS.containsKey(character)) {

And instead of peek()ing once per bracket, we do it a flat rate of once:

else if (!bracketStack.empty() &&
    MATCHING_CHARS.get(bracketStack.peek()) == character) {

Note that since get returns null when the key isn't there, it automagically doesn't give a crap if the character isn't in MATCHING_CHARS! (and also doesn't match)

And finally, we replace your last if with:

else if (MATCHING_CHARS.containsValue(character)) {

Whew! Much simpler. Now, unless you plan to synchronize that regex we used at the beginning and the map for any brackets you add in the future, I suggest we do this:

private static final String REMOVE_REGEX = "[^\\" + String.join("\\",
    MATCHING_CHARS.values().appendAll(MATCHING_CHARS.keys()) // every bracket
) + "]"; //for brackets 'a', 'b', 'c' should be "[^\\a\\b\\c]"

Then we use that in the place of the regex from before.

Actually, I don't suggest we do that awful mess of a line, I suggest we use a language where it's easier to get the behavior I'm looking for and doesn't require ugly hacks, but whatever. Ya make do with what you've got.

Now, one last tip: Don't name things string. Name it checking or toCheck or something that describes its purpose. Java's a statically-typed language; we can tell that it's a String without you naming it that.


Calculating set difference of lists with or without intermediate variables Which form of the method do you find more readable?

Version with local variables:

public void checkRetrievedIds( List<Integer> expectedIds, List<Integer> retrievedIds ) {
    Set<Integer> expected = new HashSet<>( expectedIds );
    Set<Integer> retrieved = new HashSet<>( retrievedIds );

    Set<Integer> difference = Sets.difference( expected, retrieved );
    createInvalidationEvents( difference );
}

Or the version without local variables:

public void checkRetrievedIds( List<Integer> expectedIds, List<Integer> retrievedIds ) {
    createInvalidationEvents( Sets.difference(
        new HashSet<>( expectedIds ),
        new HashSet<>( retrievedIds )
    ) );
}

I'm considering the second version as more readable as it reveals the method intent more clearly.

I'd prefer

public void checkRetrievedIds( List<Integer> expectedIds, List<Integer> retrievedIds ) {
    Set<Integer> difference = Sets.difference(
        new HashSet<>( expectedIds ),
        new HashSet<>( retrievedIds )
    );

    createInvalidationEvents( difference );
}

You're right that your first version is somewhat verbose for the task.  However, there are some good reasons to separate out the difference calculation from the function call.  First, what if you wanted to do something else with the difference?  You'd immediately have to refactor to something like this version so as to have the difference to manipulate.

I'm also somewhat leery of a function that consists entirely of chained calls.  One has to sort of mentally unwrap it in order to get to the real meaning.  They often make a lot of sense when writing them, as you know exactly what you are trying to accomplish.  They can make a lot less sense six months down the road.

Separating the two allows us to basically have one line that calculates the set difference of two lists while the other line makes use of that difference.  That seems a reasonable division of responsibility.

Another possibility would be to change the signature of createInvalidationEvents to take two lists as input.  Then you wouldn't need checkRetrievedIds.  Of course, that wouldn't work if createInvalidationEvents is used elsewhere with the current signature.

A third possibility would be to calculate the set difference in whatever calls checkRetrievedIds.  This would also get rid of checkRetrievedIds.

As written, checkRetrievedIds is simply an alias for createInvalidationEvents with a different method signature.  Without other context, it's unclear why you need it.


Redirection with params - No query string I have a products page with a search box. When a search is submitted there's a redirection to a search results page.
If another search is submitted then the query string remains intact (results are populated via ajax).
So what I've done is running twice to the server, one time using ajax to set the Session value and another for the redirect itself:

Products page

function search() {
    //get the search query text
    var query = $('#productSearchInput').val();
    //url to a method on the search controller
    var url = 'http://SearchPageUrl' + '/SetSearchQuery';
    //set the session value
    $.get(url, { query: query })
        .done(function () {
            //actual redirect
            window.location.href = 'http://SearchPageUrl';
        });
    }

Search results controller - set the session value

public void SetSearchQuery(string query)
{
    System.Web.HttpContext.Current.Session["searchQuery"] = WebUtility.HtmlDecode(query);
}

Search page entry point

public ActionResult Index()
{
    string query = System.Web.HttpContext.Current.Session["searchQuery"].ToString();
    ViewBag.query = query;

    //get the products...
    //var results = GetProducts(query);
    return View("~/Views/something/SearchResults.cshtml", results);
}

So this works but it feels like a lot of not so maintainable or efficient implementation.
How can I make it better?

Using the session to store this is icky, since a user may have multiple tabs open. I.e. multiple searches, but only one that your server remembers.

Anyway, if I understand this correctly, try this:

The first time a user hits the results page, have it pre-populated with results. Don't load anything via ajax; it should already be there. However, the results page should also have a different handler for the user clicking search/submitting the form.

Anywhere else on the site, searching should just send you to the results page. On the results page, however, the search function does its AJAX thing, and fetches the results, replacing whatever's on the page.

Now, to update the displayed URL, use pushState and listen for popState events too, to update the address bar, and keep a "correct" browsing history.

So on your results page, you'll want something like this for your search handler:

function search() {
    // get the search query text
    var query = $('#productSearchInput').val();

    // url for the search page
    var base = 'http://SearchPageUrl';

    // create the new URL to display in the address bar/history
    var url = base + '?query=' + encodeURIComponent(query);

    // fetch results for the new query
    $.get(base, { query: query })
        .done(function (data) {
            // push the url to the address bar/history
            history.pushState({query: query}, "Search results", url);

            // insert new results into the page ...
        });
}

And on any other page, you want no JS. Just a plain GET form pointing at the results page.

Alternatively, if I misunderstood and you don't want queries in the address bar at all, simply do what randcd suggested in the comments: Use POST. Make the search form's method POST, and use $.post() when you need to update results.


Writing slice compare in a more compact way I have this code snippet comparing two slice references:

  fn compare(&self, a: &[u8], b: &[u8]) -> i64 {
    let mut p1 = a.as_ptr() as *mut u8;
    let mut p2 = b.as_ptr() as *mut u8;

    let mut min_len = cmp::min(a.len(), b.len());

    let mut ret = 0;
    while min_len > 0 {
      unsafe {
        if *p1 != *p2 {
          ret = *p1 as i64 - *p2 as i64;
          break;
        } else {
          p1 = p1.offset(1);
          p2 = p2.offset(1);
        }
      }
      min_len = min_len - 1;
    }

    if ret == 0 {
      if a.len() < b.len() {ret = -1;}
      else if a.len() > b.len() {ret = 1;}
    }

    return ret;
  }

I think I wrote it down in C style. I would like to write this in Rust style.

I could replace 'ret' with std::cmp::Ordering. What else could I do?

What about this:

fn compare(a: &[u8], b: &[u8]) -> cmp::Ordering {
    for (ai, bi) in a.iter().zip(b.iter()) {
        match ai.cmp(&bi) {
            Ordering::Equal => continue,
            ord => return ord
        }
    }

    /* if every single element was equal, compare length */
    a.len().cmp(&b.len())
}

I've removed the self argument, and used the cmp-module to do the actual comparison and zip() as @alexv already suggested.


Convert string array to string with separator I have this little convenience method that takes an String array and returns an String where the values are separated by a provided separator (comma, pipe, etc).

For instance, if given an array with values "abc", "def" and "ghi", and a for separator we passed " | ", the end result would be "abc | def | ghi".

This is the method:

public String getArrayAsString(String[] stringArray, String separator) {
    StringBuilder finalString = new StringBuilder();
    boolean needsSeparator = false;
    for (String s : stringArray) {
        if (needsSeparator) {
            finalString.append(separator);
        }
        finalString.append(s);
        needsSeparator = true;
    }
    return finalString.toString();
}

Notice that the separator must, as its name implies, separate values, and not be appended or prepended artificially to the final String. So, for instance, the following would be incorrect return values: "abc | def | ghi | " or " | abc | def | ghi".

I specifically don't like the needsSeparator flag variable, but without it, the unwanted leading or trailing separators would be added, and then I would need to remove them from the finalString before returning it, which I also don't find to be very elegant.

Is there a more concise (perhaps more elegant) way to achieve this?

Before I review your code I want to point out that Java has a String.join() method which does exactly that. Now to your code:

You use StringBuilder, good. You take care of not appending a seperator after the last and not prepending before the first element, also good. You don't handle null values though, it's your decision if you want to handle them separately or just let Java just throw a NPE. For the separator however, a null appends literally null to the String, I'd rather default to "" if separator is null. A way to get rid of the flag:

public String getArrayAsString(String[] stringArray, String separator) {
    if(stringArray == null || stringArray.length == 0) return "";

With this we already ensured that our stringArray has at least one entry. What we can do now is initialize the StringBuilder with the first String:

    StringBuilder finalString = new StringBuilder(stringArray[0]);

Since all strings after the first one need to prepend the separator, we don't need the flag anymore. Unfortunately, we can't use a foreach loop, since we have to start at index 1 as the element at index 0 is already in the StringBuilder:

    for(int i = 1; i < stringArray.length; ++i)
        finalString.append(separator).append(stringArray[i]);

And then just return what we got:

    return finalString.toString();

Whole function:

public String getArrayAsString(String[] stringArray, String separator) {
    if(stringArray == null || stringArray.length == 0) return "";

    StringBuilder finalString = new StringBuilder(stringArray[0]);

    for(int i = 1; i < stringArray.length; ++i)
        finalString.append(separator).append(stringArray[i]);

    return finalString.toString();
}

Edit:

In String.join() the parameters are swapped, so the separator comes first, the array second:

    String[] arr = new String[]{"Hello", "World"};
    System.out.println(String.join(" | ", arr));
    System.out.println(getArrayAsString(arr, " | ")); //Your function

Output:

Hello | World
Hello | World

Working code at ideone.com


Java application processing GUI input when "Done" is clicked Right now, I've got this really ugly loop in my code that waits for a boolean to be true before the method can return. It's triggered when "Done" is clicked. I'm wondering, what is the proper way for me to delay this method's return until the button is pressed? How should I refactor this code so that it follows common practices for this?

Here is the relevant code. It is part of a much bigger application. This is the only place this happens because it is the only custom dialog box in the application. Everything else is done with JOptionPane, so it doesn't have this problem.

import javax.swing.*;
import java.awt.*;
import java.awt.event.*;
import java.io.*;

public class Main {
    public static void main(String[] args) throws FileNotFoundException {
        PrintWriter out = new PrintWriter("textfile.txt");
        out.println(GUI.getUserInput());
        out.close();
    }
}

class GUI implements ActionListener {

    private static boolean pressed;

    public static String getUserInput() {
        pressed = false;

        JFrame frame = new JFrame();
        frame.setSize(400, 250);
        frame.setLocationRelativeTo(null);
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.setTitle("Diary");
        JPanel panel = new JPanel();
        panel.setLayout(new BorderLayout(4, 2));
        JLabel instructionLabel = new JLabel("What's on your mind?");
        panel.add(instructionLabel, BorderLayout.NORTH);
        JTextArea inputField = new JTextArea("", 7, 30);
        inputField.setLineWrap(true);
        inputField.setWrapStyleWord(true);
        panel.add(new JScrollPane(inputField), BorderLayout.CENTER);
        JButton done = new JButton("Done");
        done.addActionListener(new GUI());
        panel.add(done, BorderLayout.SOUTH);
        frame.add(panel);
        frame.setVisible(true);
        inputField.setCaretPosition(inputField.getText().length());

        while (!pressed) { // Wait for the "Done" button to be pressed
            // For some reason, there has to be something in this loop for it to work
            System.out.println();
        }

        frame.dispose();
        return inputField.getText();
    }

    public void actionPerformed(ActionEvent e) {
        pressed = true;
    }
}

1 import

I suggest you don't use the wildcard in your import statements, because

it clutters your namespace,
the maintainer of your code does not know what classes exactly you use in your code.

2 Main class

I would rename your class Main to, say, MyFrame, and declare it as a subclass of JFrame.

Summa summarum

All in all, I had this in mind:

import java.awt.BorderLayout;
import java.awt.Dimension;
import java.awt.Toolkit;
import java.awt.Window;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.awt.event.WindowEvent;
import java.io.FileNotFoundException;
import java.io.PrintWriter;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.JScrollPane;
import javax.swing.JTextArea;
import javax.swing.SwingUtilities;

public class MyFrame extends JFrame {

    private final JPanel panel = new JPanel();
    private final JLabel instructionLabel = new JLabel("What's on your mind?");
    private final JTextArea textArea = new JTextArea(7, 30);
    private final JButton button = new JButton("Done");

    public MyFrame() {
        super("Diary"); // Sets the title to "Diary".
        this.setSize(400, 250);
        this.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        this.panel.setLayout(new BorderLayout(4, 2));
        this.panel.add(instructionLabel, BorderLayout.NORTH);
        this.textArea.setLineWrap(true);
        this.textArea.setWrapStyleWord(true);
        this.panel.add(new JScrollPane(textArea), BorderLayout.CENTER);
        this.button.addActionListener(new MyActionListener());
        this.panel.add(button, BorderLayout.SOUTH);
        this.getContentPane().add(panel);
        centerWindow(this);
        this.setVisible(true);
    }

    public static void main(String[] args) {
        SwingUtilities.invokeLater(() -> { new MyFrame(); });
    }

    private final class MyActionListener implements ActionListener {

        @Override
        public void actionPerformed(ActionEvent e) {
            try (PrintWriter writer = new PrintWriter("textfile.txt")) {
                writer.println(textArea.getText());
            } catch (final FileNotFoundException ex) {

            }

            MyFrame.this.dispatchEvent(
                    new WindowEvent(MyFrame.this, WindowEvent.WINDOW_CLOSING));
        }
    }

    private static void centerWindow(final Window window) {
        final Dimension dimension = window.getSize();
        final Dimension screen    = Toolkit.getDefaultToolkit().getScreenSize();

        window.setLocation((screen.width - dimension.width) / 2,
                           (screen.height - dimension.height) / 2);
    }
}

Hope that helps.


Organise method flow I have something like a shop. It is limited to a checkout page, which will send an email. Nothing big, but a nice thing to learn more OOP.

Almost every time I develop new classes I am wondering how to organize the flow of depending methods. Here is an example, a first draft of what I have:

<?php

// Outside the class:
if (!empty($cms->request->post('add_to_cart'))) {
    $cart->addStrt();
}

OPTION A

// OPTION A: Build a "method chain" and move from one to the other, to the next …
Class Cart extends Shop {

    // This is the entry point and starts the "add process"
    public function addStart() {
        $this->checkQuantity(); // if qty is between min/max
        return false;
    }
    // Step 2
    public function checkQuantity() {
        if ($quantity < $max && $quantity > $min) {
            $this->checkInStock(); // move on to the next step
        }
        return false;
    }
    // Step 3
    public function checkInStock() {
        if ($availabe >= $qty_in_basket) {
            $this->addToCart(); // move on to the final step
        }
        return false;
    }
    // Step 4
    public function addToCart() {
        // Final step, all requirements ok
        $product_id = $this->request->post->product_id;
        $cart = $this->getCart();
        $product = $shop->getProduct($product_id);

        $cart->items->add($product);
        $added = $cart->save();

        return $added;
    }
}

OPTION B

// OPTION B: Have a single "add" method and check all requirements, sanitize, etc from a single method
Class Cart extends Shop {

    public function add() {
        $continue = $this->checkQuantity(); // if qty is between min/max
        if ($continue == false) return false;

        $continue = $this->checkInStock(); // if product still available
        if ($continue == false) return false;

        // Perhaps more checks, sanitize, etc.

        // Everythiing mus be ok, put item into cart
        $product_id = $this->request->post->product_id;
        $cart = $this->getCart();
        $product = $shop->getProduct($product_id);

        $cart->items->add($product);
        $added = $cart->save();

        return $added;
    }
}

I have read tutorials on some OOP patterns but can't seem to find which fits to my question. While Option B seems to be cleaner (to understand/extend) one thing I always read is never put multiple tasks into one method. Every method should only handle one specific task. But how/where do I control the flow?

Is this "Encapsulation" or "Decorator pattern" or "Dependency Injection"? I have no idea what I should google.

It's good to have single method to control flow as you have in B, but this method should be mostly calling other methods of your class (you also call there checkQuantity for example, which is also in A). That way add is still very flexible - you can override those methods (probably protected methods, not public), that it calls or as last resort override that method itself.

If you think B is more readable, imagine, there would be something like this->addToCart instead of this bit (and that extracted to separate method):

   $product_id = $this->request->post->product_id;
    $cart = $this->getCart();
    $product = $shop->getProduct($product_id);

    $cart->items->add($product);
    $added = $cart->save();

I'd google for "single responsibility principle".


Testing for "bouncy" numbers I have a segment of code:

/*functions return 4 if all digits are the same, 5 if digits are decreasing, 6 if digits are increasing, 7 if number's digits are bouncy.*/
        int variable = 0;
        cout << number << endl;
        variable = sameDigitsTest(number);
        if (variable == 0)
            variable = decreasingTest(number);
        if (variable == 0)
            variable = increasingTest(number);
        if (variable == 0)
            variable = bouncyTest(number);
        /*variable will not be zero any more, hence I do not test for the value zero*/
        if (variable == 4 || variable == 5 || variable == 6){
            cout << "The number is not bouncy." << endl;
            nonBouncyCounter++;
        }
        if (variable == 7){
            cout << "The number is bouncy." << endl;
            bouncyCounter++;
        }

How can I eliminate all of the if statements to make my code more compact? In fact I was thinking of including a function that includes the functions xTest, and then returning that value and then keeping the two if statements at the end.

The use of magic numbers isn't great.  Your xTest functions should really have Boolean return values, which would allow you to remove the local storage of the result and simplify further (relying on operator short-circuiting):

void testNumber(int number) {
    if (sameDigitsTest(number) || decreasingTest(number) || increasingTest(number)) {
        nonBouncyCounter++;
    }
    else if (bouncyTest(number)) {
        bouncyCounter++;
    }
}

(Note that the latter does actually work with the current int return values, but it appears that you've only coded them that way to fit your original implementation; it's unnecessary complexity.)


Fermat primality test Here is a simple working Java implementation of primality test for Fermat numbers. Is there something that I could change in code to achieve a better running time?

import java.math.BigInteger;

public class FPT
{
    public static void main(String[] args)
    {
        double n;
        n = Double.parseDouble(args[0]);

        int e = (int)n;

        if (e > 1)
        {
            double m = Math.pow(2,n);
            int k = (int)m;
            BigInteger F;
            F = BigInteger.valueOf(2).pow(k).add(BigInteger.ONE);
            BigInteger s = new BigInteger ("8");
            double o = 1;
            double a = n - o ;
            double b = Math.pow(2,a) - o;
            int c = (int)b;

            for (int i = 1; i <= c; i ++)
            {
                 s=s.pow(4).subtract(BigInteger.valueOf(4).multiply(s.pow(2))).add(BigInteger.valueOf(2)).mod(F);
            }

            if (s.equals(BigInteger.ZERO))
            {
                System.out.println("prime");
            }
            else
            {
                System.out.println("composite");
            }
        }
        else
        {
            System.out.println("exponent must be greater than one");
        }
    }
}

I still can't quite figure out the way to this algorithm =/. But, from the Java point of view, you can gain 9-10% by changing your loop to:

for (int i = 1; i <= c; i ++) {
    BigInteger temp = s.pow(2);
    s = temp.pow(2).subtract(BigInteger.valueOf(4).multiply(temp)).add(BigInteger.valueOf(2)).mod(F);
}

...because the cached value can be used twice there.

Otherwise, the code is hard to read. It's not self explanatory nor commented and could be written in a better way - for a human to read, at least. But none of those changes would affect performance in any way.

EDIT: Additional 17% thanks to @Landei in comments.

for (int i = 1; i <= c; i ++) {
    BigInteger temp = s.modPow(BigInteger.valueOf(2), F);
    s = temp.pow(2).subtract(BigInteger.valueOf(4).multiply(temp)).add(BigInteger.valueOf(2));
}
s = s.mod(F);

That takes advantage of modPow() method which can do pow().mod() in one step without much additional overhead. Since the mod is not necessary to have every time, it is enough to do it in temp (that actually propagates twice into the resulting expression) and enjoy the substantial speedup. You need to add one proper mod after the loop is done, though.


Script for updating an Oracle database I have the following piece of Perl script for updating an Oracle database. It's working perfectly fine, but I want to know how I can simplify this script.

use DBI;
my $dbh;
my $bad_form = "text.txt";

open (OUT, $bad_form) or die "Could not open $bad_form";
while (<OUT>){
   chomp $_;
   my ($item_id, $description, $form_id_1, $form_id_3) = split(/\|/, $_);

   my $new_form_id_1 = form_1($form_id_1);
   my $new_form_id_3 = form_3($form_id_3);

   if ( $new_form_id_1 ne 'null' ){
        my $sql = "update item_table set form_id_1 = '$new_form_id_1' where item_id = '$item_id'";
        my $item = $dbh->prepare($sql) or (log_error("Couldn't prepare statement: " . $dbh->errstr) and return 2);
        $item->execute() or (log_error("Couldn't execute statement: " . $item->errstr) and return 2);
   }else{
        $form_id_1 = "correct value";
        $new_form_id_1 = "no updated";
}

   if ( $new_form_id_3 ne 'null' ){
        my $sql = "update item_table set form_id_3 = '$new_form_id_3' where item_id = '$item_id'";
        my $item = $dbh->prepare($sql) or (log_error("Couldn't prepare statement: " . $dbh->errstr) and return 2);
        $item->execute() or (log_error("Couldn't execute statement: " . $item->errstr) and return 2);

   }else{
        $form_id_3 = "correct value";
        $new_form_id_3 = "no updated";
   }
}

Some basic refactoring, and using ? placeholders in sql queries,

use strict;
use warnings;
use DBI;
my $dbh;
my $bad_form = "text.txt";

sub prepare_execute {
    my ($sql, $values, $dbh) = @_;

    my $item = $dbh->prepare($sql) or do {
        log_error("Couldn't prepare statement: " . $dbh->errstr);
        die;
    };
    $item->execute(@$values) or do {
        log_error("Couldn't execute statement: " . $item->errstr);
        die;
    };
    return 1;
}

open (my $OUT, "<", $bad_form) or die "$! $bad_form";
while (<$OUT>) {
   chomp $_;
   my ($item_id, $description, $form_id_1, $form_id_3) = split(/\|/, $_);

   my $new_form_id_1 = form_1($form_id_1);
   my $new_form_id_3 = form_3($form_id_3);

   if ( $new_form_id_1 ne 'null' ){
        my $sql = "update item_table set form_id_1 =? where item_id =?";
        eval { prepare_execute($sql, [$new_form_id_1, $item_id], $dbh) }
          or return 2;
   }
   else {
        $form_id_1 = "correct value";
        $new_form_id_1 = "no updated";
    }

   if ( $new_form_id_3 ne 'null' ) {
        my $sql = "update item_table set form_id_3 =? where item_id =?";
        eval { prepare_execute($sql, [$new_form_id_3, $item_id], $dbh) }
          or return 2;
   }
   else {
        $form_id_3 = "correct value";
        $new_form_id_3 = "no updated";
   }
}


Prettifying URLs for a PHP-based club website I have a website for my car enthusiast club ("example.org") whose content can be divided into three main sections: Startseite (the "front page" of the site), Mitglieder (membership directory), and Vereinsinfos ("about us").  For each section, the content is handled by a corresponding PHP script: /home.php, /mitglieder.php, and /verein.php.

I'd like to prettify the URLs to eliminate the .php extensions, so that the URLs look like

http://example.org/home (maps to /home.php)
http://example.org/home/ (also maps to /home.php)
http://example.org/home/1 (maps to /home.php with a data=1 query string)

After reading all documentation, I formed my .htaccess like this (repeated for the mitglieder and verein sections):

RewriteRule ^home\/$ /home.php [L]
RewriteRule ^home$ /home/ [L]
RewriteRule ^home\/([^/]*)$ /home.php?data=$1 [L]

This works, but I can't figure out how to improve it further after reading all the documentation on the Internet.  (I have difficulty understanding regular expressions.)

In addition to the content above, I also have static resources under /images/, /js/, /css/, etc. which should be left alone.

You can combine these:

RewriteRule ^home\/$ /home.php [L]
RewriteRule ^home$ /home/ [L]

Into:

RewriteRule ^home\/?$ /home.php [L]

In addition, you should also map /home.php to /home.
So this will do all three:

RewriteRule ^home(?:\/|\.php)?$ /home.php [L]

Now your /home/1 rules. You've already got it, but you can make slight improvement:

RewriteRule ^home\/([^/]+)$ /home.php?data=$1 [L]

The + differentiates this rule from /home/.

If the data is alphanumerical you can improve this further:

RewriteRule ^home\/([\w-]+)$ /home.php?data=$1 [L]

You can replace home with other URLs
as well.


Colorbar for Matplotlib 3D patch plot I am trying to make a 3D grid plot in Python using Matplotlib. The grid consists of hexahedral cells (with qudrilateral surfaces). The surfaces should be colored according to their height, that is: the z coordinate. A colorbar should accompany the plot.

Here is an example plotting three surfaces:

from matplotlib import pyplot as plt
from matplotlib import cm
from mpl_toolkits.mplot3d.art3d import Poly3DCollection
import numpy as np

def getFaceMidpointsZ(pc):
    a=[]

    for i in pc:
        v=0
        for j in i:
            v=v+j[2]
        v=v/4.
        a.append(v)
    return a

def getColors(m,a):
    b=m.to_rgba(a)
    return [(i[0],i[1],i[2]) for i in b]

fig = plt.figure()
ax = plt.axes(projection='3d')
P=[(0,0,0),(2,0,0),(2,2,0),(0,2,0),(0,2,2),(2,2,2),(0,0,2)]
F=[[0,1,2,3],[2,3,4,5],[0,3,4,6]]

x = [t[0] for t in P]
y = [t[1] for t in P]
z = [t[2] for t in P]

pc=[[P[i] for i in f] for f in F]

fz=getFaceMidpointsZ(pc)

q = Poly3DCollection(pc, linewidths=1)
m = cm.ScalarMappable(cmap=cm.jet)
m.set_array([min(z),max(z)])
m.set_clim(vmin=min(z),vmax=max(z))

q.set_facecolor(getColors(m,fz))
ax.add_collection3d(q)

fig.colorbar(m)

ax.set_xlabel('x')
ax.set_ylabel('y')
ax.set_zlabel('z')
ax.set_xlim(min(x),max(x))
ax.set_ylim(min(y),max(y))
ax.set_zlim(min(z),max(z))

ax.view_init(elev=18., azim=-43.)
plt.show()

I am starting to learn Python and Matplotlib, so any comments would be appreciated.

Coding style

There is an official coding style guide for Python called PEP8. Give it a good read and follow it. For example, the most glaring violations:

Spacing around operators: instead of v=v+j[2], write as v = v + j[2]
For function names, snake_case is preferred over camelCase
Put exactly two blank lines before each function definition

Coding practices

Don't put code in the global namespace, put inside a function.
This protects you from bad practices like using global variables, whether intentionally or by accident.

For example, you could move all the code current code in the global namespace inside a main function:

def main():
    fig = plt.figure()
    ax = plt.axes(projection='3d')
    P = [(0, 0, 0), (2, 0, 0), (2, 2, 0), (0, 2, 0), (0, 2, 2), (2, 2, 2), (0, 0, 2)]
    F = [[0, 1, 2, 3], [2, 3, 4, 5], [0, 3, 4, 6]]
    # ...

And call the main function like this:

if __name__ == '__main__':
    main()

Avoid unused imports:

import numpy as np

Naming

The biggest issue with this code is the naming of variables.
You're overusing single-letter or very short variable names,
and it's hard to guess what is what without searching for where they come from and how they are used.

Simplify

You have many expressions that can be simplified, for example:

# instead of: v = v + j[2]
v += j[2]

# instead of: v = v / 4.
v /= 4.


Thread safe reader-writer queue with C++11 Below is my honest attempt to write thread safe queue using C++11 features. Please review it and provide your comments.

#include <queue>
#include <condition_variable>
#include <atomic>
#include <iostream>

template <typename T>
class ThreadSafeQueue
{
    std::queue<T> m_queue;
    std::mutex m_mutex;
public :

    void Push(const T& data)
    {
        std::lock_guard<std::mutex> lock(m_mutex);
        m_queue.push(data);
    }

    // Universal reference
    template <typename ...Args>
    void Emplace(Args&&... args)
    {
        std::lock_guard<std::mutex> lock(m_mutex);
        m_queue.emplace(std::forward<Args>(args)...);
    }

    void PopFront(T& data)
    {
        std::lock_guard<std::mutex> lock(m_mutex);
        data = m_queue.front();
        m_queue.pop();
    }

    void PopFrontNoLock(T& data)
    {
        data = m_queue.front();
        m_queue.pop();
    }

    size_t Size()
    {
        std::lock_guard<std::mutex> lock(m_mutex);
        return m_queue.size();
    }

    bool Empty()
    {
        std::lock_guard<std::mutex> lock(m_mutex);
        return m_queue.empty();
    }

    bool EmptyNoLock()
    {
        return m_queue.empty();
    }

    std::mutex& GetMutex()
    {
        return m_mutex;
    }

};

template <typename T>
class ReaderWriterQueue
{
    ThreadSafeQueue<T> m_safeQueue{};
    std::condition_variable m_condVar{};
    std::atomic<bool> m_writeCompleted{};
public :
    void Push(const T& data)
    {
        m_safeQueue.Push(data);
        m_condVar.notify_one();
    }

    // Universal reference
    template <typename ...Args>
    void Emplace(Args&&... args)
    {
        m_safeQueue.Emplace(std::forward<Args>(args)...);
        m_condVar.notify_one();
    }

    void PopFront(T& data)
    {
        std::unique_lock<std::mutex> lock(m_safeQueue.GetMutex());
        m_condVar.wait(lock, [this] {return (!m_safeQueue.EmptyNoLock() || m_writeCompleted); });
        m_safeQueue.PopFrontNoLock(data);
    }

    void NotifyWriteEnd()
    {
        m_writeCompleted = true;
    }

    bool IsWriteFinished() const
    {
        return m_writeCompleted;
    }

    bool Empty()
    {
        return m_safeQueue.Empty();
    }

};

Driver code to test thread safe queue:

void ProduceData(ReaderWriterQueue<int>& queue)
{
    int i = 0;
    while (i < 10)
    {
        queue.Emplace(i);
        i++;
    }
    queue.NotifyWriteEnd();
    std::cout << "Finished Writing" << std::endl;
}

void ReadData(ReaderWriterQueue<int>& queue)
{
    while (!queue.IsWriteFinished() || !queue.Empty())
    {
        int waitTime;
        queue.PopFront(waitTime);
        std::cout << "popped " << waitTime << "\n";
        std::this_thread::sleep_for(std::chrono::seconds(waitTime));
    }
    std::cout << "Finished Reading" << std::endl;
}

int main()
{
    ReaderWriterQueue<int> rwQueue;
    std::thread writer(ProduceData, std::ref(rwQueue));
    const int readerThreadCount = 2;
    std::thread readers[readerThreadCount];
    for (int count=0; count < readerThreadCount; count++)
    {
        std::thread reader(ReadData, std::ref(rwQueue));
        readers[count] = std::move(reader);
    }

    if (writer.joinable())
    {
        writer.join();
    }
    for (int count = 0; count < readerThreadCount; count++)
    {
        if (readers[count].joinable())
        {
            readers[count].join();
        }
    }
    return 0;
}

There's no way to notify locked consumers of the write end. You might want to modify PopFront and NotifyWriteEnd to return a special null value to every awaiting consumer (or otherwise wake up all the awaiting consumers and notify them the queue is exhausted) when NotifyWriteEnd() is called. Otherwise you'll need a rather serious contract inherent in your application design, that every consumer should know in advance that it would be able to get a matching producer before starting awaiting on the queue.


Is my ObjectCache wrapper sound? Wrapper:

internal static class Cache
{
    private static ObjectCache InternalCache
    {
        get
        {
            return MemoryCache.Default;
        }
    }

    private static T CacheOrGetExisting<T>(string key, Func<T> valueFactory)
    {
        T value;
        object uncastedValue = InternalCache.Get(key);

        if (uncastedValue == null)
        {
            value = valueFactory();
            InternalCache.Set(key, value, DateTimeOffset.UtcNow.AddHours(1));
        }
        else
        {
            value = (T)uncastedValue;
        }

        return value;
    }

    public static decimal Foo
    {
        get
        {
            return CacheOrGetExisting("foo", () =>
                {
                    return 5m; // data access here
                });
        }
    }

    public static Bar Bar
    {
        get
        {
            return CacheOrGetExisting("bar", () =>
                {
                    return new Bar(); // data access here
                });
        }
    }
}

Usage in a Controller:

model.Foo = Cache.Foo;

One thing I'm not sure of: is the readonly property necessary for InternalCache or can I write private static ObjectCache InternalCache = MemoryCache.Default;?

I'm not worried about the tiny chance of multiple calls to the same uncached object causing multiple value = valueFactory(); and I'm also not worried about stale data. The main goal is to cache some informational data that is simply displayed on the screen, instead of retrieving it every request.

I've tested this with an expiration of 1 minute and a class that gets DateTime.Now on creation, and it worked fine, so I believe the code is bug-free.

As Magus pointed out in the comments, making everything static can present some issues. Using these static properties in other classes makes those classes tightly coupled to the Cache class. Why should you care? Consider this class:

public class Bar
{
    public int DoSomething()
    {
        return 5 + Cache.CachedNumber;
    }
}

You've lost your ability to test DoSomething() in isolation from the rest of your code. Any time you test DoSomething(), it relies on a specific implementation of Cache. It also makes it difficult to use the Bar class with different cache types (in memory, database, file system, etc.). Ideally, Bar would look something like this:

public class Bar
{
    private ICacheProvider Cache { get; set; }

    public Bar(ICacheProvider cache)
    {
        this.Cache = cache;
    }

    public int DoSomething()
    {
        return 5 + Cache.CachedNumber;
    }
}

Now we can:

Test that DoSomething is doing its job, regardless of flaws in the ICacheProvider instance (by passing a mock ICacheProvider that always returns CachedNumber = 3, for example).
Pass in different ICacheProvider implementations that can very depending on my use case, and Bar will still work without caring about them.

Also, I would suggest not using something as vague as Cache or ICacheProvider (used for illustration). Once you start caching a lot of things, you may want to cache different properties differently, at which point your super cache class will become too large to maintain. Consider breaking it up into meaningful repositories that can implement caching however they want, or something similar.


Computing the Kronecker product over a tensor chain I'm working on some Python code and have a few functions which do similar things, and the only way I've found of writing them is quite ugly and not very clear.
In the example below, the goal is to compute the Kronecker product over a tensor chain of length M, in which the mth tensor is R and every other tensor is J.
Is there any nice way to rewrite this?
def make_rotate_target(m, M, J, R):
    out = J
    if M == 1:
        return R
    else:
        for i in range(M):
            if i == 0:
                out = J
            else:
                if i + 1 == m:
                    out = np.kron(out, R)
                else:
                    out = np.kron(out, J)
    return out

Assuming M is non-negative you can make it clear when J and R are returned and remove some of the nested if-else's.
Since if i+1 ==m only evaluates as true if m <= M you could check that and use different for loops if it's true/false to make things slightly faster but I think that would decrease readability for not much gain
def make_rotate_target(m, M, J, R):
    if M == 0:
        return J

    if M == 1:
        return R

    out = J
    for i in range(1, M):
        if i + 1 == m:
            out = np.kron(out, R)
        else:
            out = np.kron(out, J)

    return out


Backend interface VS hard contract Here is an API for using a warehouse:

public static class WarehouseClient
{
    event Action<IWarehouse> WarehouseCreated;
}

public interface IWarehouse
{
    event Action<IWarehouseSession> NextSession;

    IWarehouseSession Session { get; }

    WarehouseInfo Info { get; }
}

public interface IWarehouseSession
{
    event Action Opened;

    event Action Closed;

    bool IsOpen { get; }

    TariffPolicy TariffPolicy { get; }

    IEnumerable<ITerminal> Terminals { get; }
}

public interface ITerminal
{
    event Action<double> FreeSpaceUpdated;

    string TerminalLetter { get; }

    double FreeSpace { get; }

    IDeposit Deposit(object item);
}

public interface IDeposit
{
    object Item { get; }

    event Action ItemAccepted;

    event Action<int> ItemDeposited;

    event Action ItemWithdrawn;

    void Withdraw();
}

(WarehouseInfo and TariffPolicy are immutable value-like classes)

Also I've got third-party connections to warehouses with interfaces adaptable to mine. But I doubt if I need a mirrored API for connection wrappers, like:

public static class WarehouseProvider
{
    IWarehouseBackend CreateWarehouse(WarehouseInfo info);
}

public interface IWarehouseBackend
{
    IWarehouseSessionBackend Session { get; }

    WarehouseInfo Info { get; }

    IWarehouseSessionBackend NextSession(TariffPolicy tp, params string[] terminalLetters);
}

public interface IWarehouseSessionBackend
{
    bool IsOpen { get; }

    TariffPolicy TariffPolicy { get; }

    IEnumerable<ITerminalBackend> Terminals { get; }

    void Open();

    void Close();
}

public interface ITerminalBackend
{
    // ... and so on

This backend-api can be implemented by library's internal classes.

Removing it means much less code (though it almost written) and harder-to-implement contract for warehouse providers.

So, my question is: do I need backend api?

Also, I would be very grateful for reviewing this API.

You definitely want to document those interfaces.
WarehouseClient should probably not be a static class.
The name of the NextSession event in IWarehouse doesn't really mean anything to me. Just from reading the interface I don't really know when this event would be raised. Maybe NextSessionCreated or NextSessionOpened?
CurrentSession would be a better name instead of Session.
Also it is not clear if the Session can get opened and closed multiple times. If a session can't be opened and closed multiple time then maybe use Start and Finalize instead? Those imply (at least to me) more of a once only action.
In IDeposit: ItemWithdrawed should be ItemWithdrawn
I'm not 100% sure about the ITerminal and IDeposit relation. The way I read is that a client can deposit an object into a terminal which will then give him some kind of reference to that transaction in form of an IDeposit. Now IDeposit also has a Withdraw() method but it's not clear what the semantics of that is. Can the client who deposited the item basically withdraw that deposit? Or is it mean to be withdrawn by someone else - if yes then shouldn't there be a Withdraw() method on the terminal rather than on the deposit? It's probably not clear to me because I don't know what the workflow is supposed to be but it's hard to deduce from just the interfaces alone.

All that being said I'd say: Yes you want that backend API. Apparently the whole system expects a certain kind of workflow and providing a structure which an implementer can fill in will make it much easier for them and probably reduce bugs.


InsertSort F# program I have written this InsertSort program in F#.  It works, but I am not very happy with it.

let rec SubArrayBegin list n =
    list |> Seq.ofList |> Seq.take n |> List.ofSeq

let SubArrayEnd list n =
    List.rev (SubArrayBegin (List.rev list) ((List.length list) - n - 1))

let InsertSort list =
    let rec innerInsertSort list n =
        if n = (List.length list) then
            list
        else
            let key = List.nth list n
            let firstList = SubArrayBegin list n
            let (firstListA, firstListB) = List.partition (fun x -> x < key) firstList
            let remList = SubArrayEnd list n
            innerInsertSort (firstListA @ [key] @ firstListB @ remList) (n + 1)
    innerInsertSort list 1

[<EntryPoint>]
let main args =
    let a = [10; 2; 9; 1; 4; 5; -1]
    let b = InsertSort a
    List.iter (fun x -> printfn "%i" x) b
    0

List of annoyances:

@ symbol for joining the lists back.
If Condition being used instead of pattern matching.

I wonder if there is a better way of writing this.

Can you please review this and help me improve this code?

Function names in F# are usually written in camelCase. (Though members that are not private should use PascalCase, to adhere to general .Net naming conventions.)
SubArrayBegin is not recursive, so it doesn't need to be rec.
List implements Seq, so you don't need that Seq.ofList.
Collection manipulation functions usually have that collection as the last parameter, so that you can use them with |>.
Your implementation of SubArrayEnd is pretty complicated and inefficient. Instead, you could write it for example like this:

let rec subArrayEnd n list =
    match (n, list) with
    | 0, _ | _, [] -> list
    | _ -> subArrayEnd (n-1) (List.tail list)

There is no reason why your innerInsertSort has to follow the normal imperative version of insert sort so closely and operate on a single list (and use List.nth). Instead, you could have two collections: one where you take from using pattern matching and one where you insert using List.partition and @.
Alternatively, you could use ImmutableList from Microsoft.Bcl.Immutable instead of List for the collection you're inserting into. With that, you can insert into the middle in O(log n) and you can also find the index to insert to in O(log n) using binary search. So it will make your code simpler and also much faster.


Structuring code that shells out to an external script This code that shells out to an external script could use some input in order to read smoother, I think.  Praise and critique are welcome.

module CompanyLib
  class S3

    attr_reader :output

    def set_path local, remote
      @path = [local, remote]
    end

    def get_bucket
      /(^[^\/]*)/.match(@path[1])[0]
    end

    def sync
      @output = `aws s3 sync #{@path[0]} s3://#{@path[1]} --acl public-read`
      raise Exception, "S3 sync command failed: #{output}" unless $?.success?
    end

    def parse
      self.sync
      normalize = output.gsub("\n", "\\n")
      normalize.scan(/s3:\/\/#{self.get_bucket}(.*?)\\n/).flatten
    end

    def sync_out
      self.parse
    end
  end
end

Overall looks like a good start, some thoughts I had:

If I were implementing this class I probably wouldn't just store the output as a string, I'd parse it in a way that callers would be able to use it effectively.  For example, does it output total time, data transferred, files copied, etc.?  If you're going to try to use this data, I think this class (or a class used by this class) should have the responsibility to process that data, not the caller.
Some prefer the system() method over in-line backticks, for readability, but that's preference.
Never raise Exception, either raise RuntimeError by omitting the first parameter to raise or fail, or create a new exception class (with RuntimeError as a parent) for your purpose.  Similarly you should (in almost all cases) never rescue Exception.
In one of the codebases I work on were have an entire class dedicated to executing commands locally.  One reason we do this is that you can't isolate stdout from stderror using backticks or system(), so we use the popen4 library and generate a Result class that has the exit code, stdout and stderr.  The stderr may be useful if you're trying to relay to the caller what failed exactly.  The caller may want to retry a timeout, but not an authentication issue, for example.

I don't know how widely used or important this class is for your company, so feel free to file any of these under "over-engineering" as appropriate :-).


Singleton class to contain a DocumentClient instance, that can be overridden for testing The DocumentClient is an object that accesses CosmosDb, but Microsoft recommends its kept as a Singleton class. I'm relying on System.Lazy to ensure thread safety.

In addition, I want to be able to overwrite my DocumentClient when I need to test. Is this correct?

public sealed class DocumentClientSingleton
{
    public static IDocumentClient GetInstance()
    {
        return _lazy.Value;
    }

    /// <summary>
    /// Only use this for testing!
    /// </summary>
    /// <returns></returns>
    public static IDocumentClient SetInstanceForTesting(IDocumentClient client)
    {
        _lazy = new Lazy<IDocumentClient>(() => client );
        return _lazy.Value;
    }

    private static  Lazy<IDocumentClient> _lazy =
        new Lazy<IDocumentClient>(InstantiateDocument);

    private static IDocumentClient InstantiateDocument()
    {
        return new DocumentClient(
            new Uri(ConfigurationManager.AppSettings["endpoint"]),
            ConfigurationManager.AppSettings["authKey"],
            new ConnectionPolicy
            {
                ConnectionMode = ConnectionMode.Direct,
                ConnectionProtocol = Protocol.Tcp
            }
        );
    }
}

This answer only has one tiny section (IDisposable) that I feel is an appropriate kind of answer for Code Review. The rest is more suitable as a StackOverflow answer (since it dives into the specifics of the DocumentClient class). I apologize in advance, and if I should move my answer or edit it in anyway, please feel free to advise me in the comments section.

The DocumentClientclass is already thread-safe.

From what I can gather reading the latest documentation on the DocumentClient class, is that it is already inherently thread-safe. Provided that this remains true, your class wrapper is obsolete.

It seems that this confusion was also asked as a question over at StackOverflow

Microsoft's recommendation of using DocumentClient as a singleton

  If you can draw a parallel to how Microsoft recommends usage of the HttpClient then you might have a better idea of what I mean from this point onwards.

Microsoft recommends that you use a single instance* per application. (again, think of HttpClient)

In the case of a web application using ASP.NET this usually comes in the form of dependency injection via declaring the lifetime of the class to Singleton.

// dependency injection for ASP.NET
services.AddSingleton(myDocumentClientInstance);

This might be the root cause of your misunderstanding. The use of the word "singleton" from the article you probably read, is actually an ASP.NET convention for specifying a lifetime of the instance and also ensuring that only a single (shared) instance is used across your entire web application.

This doesn't mean that implementing the DocumentClient as a singleton class is wrong.

In such a case, a static instance (like you have done) could be the answer. But you're going to get a group of developers that will throw fists in the air at you for making the code difficult-to-impossible to unit test if you do so... You have been warned!

The DocumentClient class implements IDisposable.

Based on your code it will become quite a complex task of making sure that the instance is properly disposed.

The recommended way to handle an object that implements the IDisposable interface is by using the the using block.

using (IDocumentClient client = new DocumentClient(new Uri("endpoint"), "authKey"))
{
    ...
}

Special cases

To summarize with a special case, which could hopefully shine some additional light why your implementation will be troublesome as a true singleton.

There are some special cases for using more than one instance. In particular, if you have multiple CosmosDb accounts, then you should actually use one instance for each account. You can read about that here.

  Note: The parallel to HttpClient does not work here, since we are creating multiple instances on purpose.

Your particular implementation will fall short when you have a scenario with multiple CosmosDb accounts. So please keep that in mind.


Organise site's module I was wondering about on how to organise the code on my site, to make it clearer as possible and clearly readable.

I'm organising it with namespace, separating every single section of the page.

For example:

var Header = {
//code relative to the header
}

var CentralBody = {
//code relative to the CentralBody
}

$(function(){
//code to execute on doc ready

});

First of all, I would know if it is a good way to organise code. In second instance, I was thinking about "what must be placed where". I explain: If, for example, I have to assign some tooltip or to bind event with code like

$('someclass').tooltip()
$('someclass').on( 'event' , ... );

where do I have to place this kind of "functions"? Do I have to place them in the concerning namespace?  Do I have to place them on document.ready?  Or writing functions in namespace and then call them on document.ready?

A conceptual question, those are always tricky.

Organizing by namespace is not a bad idea, though I would lowercase your namespace ( Header -> header etc. )

Furthermore, I would suggest to look into Movel View Controller and let each namespace have a model, view and controller.

The binding of events is in the realm of the controller. So when you initialize the controller, that is where can you do all the binding.

So in document.ready you can call centralBody.init() which could call centralBody.controller.init().