Refactored nearly identical HTML forms I have two forms in my HTML page. Both share a set of input elements (not a part of any of the forms, but rather residing separately). Besides that, the forms have their own input elements as well. Each of them also have their own notification div.

The process of POST request for both the forms is nearly identical, except those specifically owned input elements and notification divs.

Here is what I did to remove the code duplication:

function execute(config) {
    var initiator_token = getQueryVariable(window.location.search.substring(1), 'token');
    if (!initiator_token) {
        config.notifyEle
            .removeClass('alert-info')
            .addClass('alert-warning')
            .attr('data-i18n', config.invalidTokenMessage);
        return;
    }

    var params = {
        'initiator_token': initiator_token,
        'recipient_email': config.emailEle.val()
    };

    var lang = $('#mailLangSelector').val();
    if (lang) params['lang'] = lang;

    var message = $('#message').val();
    if (message) params['message'] = message;

    resetInputs();

    var loc = window.location;
    var url = loc.protocol + '//' + loc.host + $SCRIPT_ROOT + '/social' + config.url;

    $.ajax({
        url: url,
        type: 'POST',
        data: JSON.stringify(params),
        contentType: 'application/json',
        success: function(data, statusText, xhr) {
            config.notifyEle
                .removeClass('alert-warning')
                .addClass('alert-info')
                .attr('data-i18n', config.successMessage)
                .i18n();
        },
        error: function(xhr, statusText, errorThrown) {
            config.notifyEle
                .removeClass('alert-info')
                .addClass('alert-warning')
                .attr('data-i18n', config.failureMessage)
                .i18n();
        }
    });
}

So, the function accepts a config object, in which I pass form-specific values AND elements (I mean, jquery elements).

A config is like:

var inviteFormConfig = {
    emailEle: $('#inviteEmail'),
    notifyEle: $('#inviteInfo'),
    invalidTokenMessage: 'sharePage.descriptions.validTokenRequired',
    url: '/invitation',
    successMessage: 'sharePage.descriptions.invitationSent',
    failureMessage: 'sharePage.descriptions.invitationFailed'
};

And the usage for a given form:

$('#inviteForm').submit(function(evt) {
    evt.preventDefault();
    execute(inviteFormConfig);
});

Now, though this works well for my use, I want to know if there lies a better pattern for such a scenario.
Also, should I be passing around jQuery elements like this in config?

This looks quite good, one nitpick I have is that you spend a significant amount of lines showing messages, that should be a more generic information setting function like

function notifyUser( messageClass, message ){
  config.notifyEle
        .removeClass( 'alert-info alert-warning' )
        .addClass( messageClass )
        .attr('data-i18n', message );
}

And I must warn you that not putting curly braces is playing a bit with fire: if (lang) params['lang'] = lang; at least use a new line?

Edit:
I would go for an MVC approach, create a Model object that knows what fields are on the form and knows how to validate them. Then a View object that knows how to style and update the form ( put red borders around blank input types, show messages etc. ) and a a Controller object which puts all the listeners in place and also takes care of working with the model to validate the data on the form before submitting web requests.


JavaScript event handlers, scope, and the module design pattern I've been using JavaScript for some time now, but mostly just jQuery to prettify web pages.  I've started doing some serious development with it in the last year.

So, I want to make sure I'm not doing something stupid.  I found a post about the module pattern, and it makes sense to me, so I'm going to attempt to use it on this project instead of keeping nearly everything global.

I'm just a little fuzzy on where I should put event listeners.  Is there going to be any issues with scope if the listener is instantiated inside a method?  If I'm building something like a toolbar with different functions (kind of like a paint program with 'move', 'select', 'paintbrush', and 'eraser' options), do I keep the onClick for all of those in global scope?

PROJ.namespace("easel");
PROJ.easel.canvas = function() {
  //private variable to store current state
  var current_mode = "select";

  return {
    enableSelect: function () {
      //disable other tools, enable 'select' event listeners
    }
    enablePaintbrush: function () {
      //disable other tools, enable 'paintbrush' event listeners
    }
    enableMove: function () {
      //disable other tools, enable 'move' event listeners
    }
    enableEraser: function () {
      //disable other tools, enable 'eraser' event listeners
    }
  };
}();

//event listeners for the tool pallete
$('#tools #paintbrush').on('click', function() { PROJ.easel.canvas.enablePaintbrush() };
$('#tools #move').on('click', function() { PROJ.easel.canvas.enableMove() };
$('#tools #select').on('click', function() { PROJ.easel.canvas.enableSelect() };
$('#tools #eraser').on('click', function() { PROJ.easel.canvas.enableEraser() };

Of course, once the specific tool is selected, it needs its own listeners.  For example, a paintbrush needs handlers for 'mousedown' (start painting), 'mousemove' (lay down paint), and 'mouseup' (stop painting).  Can those listeners be enabled inside of PROJ.easel.canvas?

EDIT: How are you supposed to pick an "answer" on codereview.stackexchange.com?  You're asking peoples' opinions...

The way I like to use the module partern is in combination with the Oberserver pattern. I've included detailed comments in the code to help explain the two a bit. I would highly recomend you watch this video by Jeffery Way. He does a really nice job of explaining the two and also some custom events, which I think you might be interested in. Also read the links he puts under the video, they'll provide you with a complete understanding of it all.

I explain most of it in the code with comments. Anyways the following code will help you get the picture:

(function ( $, window, document, undefined ) {
   "use strict";

    var PROJ = {
        init: function() {
            //Set up my Observer Pattern
            var o = $( {} );
            $.subscribe = o.on.bind(o);
            $.unsubscribe = o.off.bind(o);
            $.publish = o.trigger.bind(o);

            //By using the object PROJ you can reference it with "this" (i.e. this.whatever();), easy peazy "namespacing"
            //Example:
            this.setUpListeners();
        },

        setUpListeners: function() {
            $('#tools #paintbrush').on('click', function() { PROJ.enablePaintbrush() };
            $('#tools #move').on('click', function() { PROJ.enableMove() };
            $('#tools #select').on('click', function() { PROJ.enableSelect() };
            $('#tools #eraser').on('click', function() { PROJ.enableEraser() };

            //Here is where the Observer Pattern kicks in nicely
            //I'm done with the listeners, and I'm letting everyone that is subscribed know that.
            //You can also namespace easily with "/" like so: ("enable/Select") or ("enable/Move") etc.
            $.publish( "listeners/are/set/up" );
        },

        enableSelect: function() {
           //do some stuff

           //I'm done with "enable", and I'm letting everyone that is subscribed know that.
           $.publish( "enable" );
        },

        enablePaint: function() {
           //do more stuff

           //When enable is published, run enableMove
           $.subscribe( "enable", this.enableMove( //You can pass arguments here to other functions, limiting the scope );
        },

        enableMove: function() {
            //In a callback "this" refers to the element in question, and not to the PROJ object

            //You can cache it out here:
            var self = this

            $("#something").on( "click", function() {
                //To resolve that you can also cache the reference in here
                var self = PROJ

                //this will work
                self.enableMove();

                //this will not work because we are in a callback
                this.enableMove();

                //You can use your Pub/Sub model anywhere, even in callbacks
                $.publish( "done/with/this/baby" );
            });
        },

        enableEraser: function() {
            $.subscribe( "done/with/this/baby", this.enableSelect() );
            //Just like that I've created a loop, by calling enableSelect it goes back and runs it again.
        } //The last closing braquet doesn't have a comma
    }; //Close the PROJ object

    //Initialize the whole thing. Can be referenced anywhere in your code after it has been declared.
    PROJ.init();
})( jQuery, window, document );

Hopefully that provided some insight. If there's any part which needs clarification I'll be happy to explain.


Collection base on year Can someone please review the code and suggest any improvements / changes? Note that this method works but wanted to see if there is a better way to reset the currentYearMeetingCount to -1. I just want to print the data if there is data for that year and for the past 3 years.

 class Program
    {
        static void Main(string[] args)
        {
            List<Meeting> Meeting = new List<Meeting>();
            Meeting.Add(new Meeting() { Name="a", Year = 2019 });
            Meeting.Add(new Meeting() { Name="b", Year = 2019 });
            Meeting.Add(new Meeting() { Name = "c", Year = 2019 });
            Meeting.Add(new Meeting() { Name = "d", Year = 2018 });
            Meeting.Add(new Meeting() { Name = "e", Year = 2018 });
            Meeting.Add(new Meeting() { Name = "f", Year = 2017 });
            Meeting.Add(new Meeting() { Name = "g", Year = 2016 });
            Meeting.Add(new Meeting() { Name = "h", Year = 2015 });
            //string Ids = peopleList.Select(x => x.ID);

            int totalYearCount = 3;
            DateTime aDate = DateTime.Now;

            int currentYearMeetingCount = Meeting.Where(x => x.Year == aDate.Year).Count();

            if (currentYearMeetingCount == 0)
            {
                aDate = DateTime.Now.AddYears(-1);

            }

            foreach (var year in Enumerable.Range(aDate.Year - totalYearCount + 1, totalYearCount).Reverse())
            {

                var meetingList = Meeting.Where(x => x.Year == year);
                foreach (var meeting in meetingList)
                {
                    Console.WriteLine("{0}-{1}", meeting.Name, meeting.Year);
                }

            }
        }

        class Meeting
        {
            public int Year { get; set; }
            public string Name { get; set; }
        }
    }

There is no need for this

DateTime aDate = DateTime.Now;

int currentYearMeetingCount = Meeting.Where(x => x.Year == aDate.Year).Count();

if (currentYearMeetingCount == 0)
{
    aDate = DateTime.Now.AddYears(-1);

}

if the collection has no data for the current year, it'll only get the past 3 years if any.  so, you can do this directly :

var totalYearCount = 3;
var maxYear = DateTime.Now.Year;
var minYear = maxYear - totalYearCount;

var result = Meeting.Where(x=> x.Year >= minYear && x.Year <= maxYear).ToList();

if(result.Count == 0)
{
    // no data, do something
}

foreach(var meeting in result)
{
    Console.WriteLine($"{meeting.Name}-{meeting.Year}");
}

this will output 2017-2019 records. However, if your logic has a special case, let's say if there is no records for current year, then start with the last year instead so the output will be from 2016 to 2019. If this is the case, then instead of depending on DateTime.Now.Year you can get the max year from the collection, and subtract the past x years from it. Something like this :

var totalYearCount = 3;
var maxYear = Meeting.Max(x=> x.Year);
var minYear = maxYear - totalYearCount;
var result = Meeting.Where(x=> x.Year >= minYear && x.Year <= maxYear).ToList();

if(result.Count == 0)
{
    // no data, do something
}

foreach(var meeting in result)
{
    Console.WriteLine($"{meeting.Name}-{meeting.Year}");
}

this will output 2016-2019 records, and if there is any records for the current year (say 2020), then it'll output 2017-2020.


Using Dynamic Programming to reduce time complexity Input

A long string of numbers, A list of numbers in string

Output

Minimum space needed in long string to match maximum numbers from list

Example

minSpaces('3141592653589793238462643383279',['314','49','15926535897','14','9323','8462643383279','4','793'])
will return 3

I am using below code to solve it, but I am not able to figure out how can I use DP for more efficient time complexity.

Thanks for your help.

dpArr = {} #empty list for DP

def findMinSpaces(k, piStr, favNumArr):
    curr = ''
    ans = 0
    N = len(piStr)
    if( k == N):
        return 0
    for i in range(k, N):
        print(curr)
        curr += piStr[i]
        if(curr in favNumArr and i != N-1):
            if(curr not in dpArr):
                ans += 1 + findMinSpaces(i+1, piStr, favNumArr)
                dpArr[curr] = ans
            else:
                print("from dpArr")
                return dpArr[curr]
    print(dpArr)
    return ans

def minSpaces(piStr, favNumArr):
    return findMinSpaces(0, piStr, favNumArr)
print(minSpaces('3141592653589793238462643383279',['314','49','15926535897','14','9323','8462643383279','4','793']))

'''
The Problem can be thought as string pattern matching, Where output will be minimum no of spaces in bigger string(piStr) to match maximum no of strings from list of smaller strings(favNumArr).
To solve this, we take one var "ans" to store no spaces and one variable "curr" to store the current pattern.
Now we iterate through the piStr and whenever we encounter(ith pos) that curr pattern is in favNumArr, we use recursion and call findMinSpaces for i+1 and increment ans with 1.
There is no need to use DP if we return from the loop with first occurrence of match and hence the loop will not run after it return value of recursion call.
The last return statement is to counter when i == N-1 when we reach the end of piStr.
The time complexity for this solution is O(n)
Any suggestion for further enhancement or if  breaks any edge case is open.'''

def findMinSpaces(k, piStr, favNumArr):
    curr = ''
    ans = 0
    N = len(piStr)
    if( k == N):
        return 0
    for i in range(k, N):
        print(curr)
        curr += piStr[i]
        if(curr in favNumArr and i != N-1):
            ans += 1 + findMinSpaces(i+1, piStr, favNumArr)
            return ans
    return ans

def minSpaces(piStr, favNumArr):
    return findMinSpaces(0, piStr, favNumArr)

print(minSpaces('',['3149v','40x9','15926535c897','1c4','932c3','84626c43383279','4c793']))


Testing PersonFirstName class as a Value Object For reference:

  A Value Object is an immutable type that is distinguishable only by the state of its properties. That is, unlike an Entity, which has a unique identifier and remains distinct even if its properties are otherwise identical, two Value Objects with the exact same properties can be considered equal.

Please see the code below:

public class PersonFirstName
{
    //public string FirstName { get; set; }

    private string _firstName;

    public PersonFirstName(string firstName)
    {
        if (firstName == null)
            throw new ArgumentNullException("Surname");
        if (!PersonFirstName.IsValid(firstName))
            throw new ArgumentException("Invalid value.", "Surname");
        this._firstName = firstName;
        FirstName = firstName;
    }

    public PersonFirstName() //for NHibernate
    {
    }

    public static bool IsValid(string candidate)
    {
        if (string.IsNullOrWhiteSpace(candidate))
            return false;
        if (candidate.Length > 255)
            return false;
        return true;
    }

    public string FirstName
    {
        get { return _firstName; }
        private set
        {
            _firstName = value;
        }
    }

    public override bool Equals(object obj)
    {
        PersonFirstName PersonFirstName = obj as PersonFirstName;
        if (ReferenceEquals(PersonFirstName, null))
            return false;
        return _firstName == PersonFirstName.FirstName;
    }

    public override int GetHashCode()
    {
        return _firstName.GetHashCode();
    }
}

and the Unit Test below:

[Test]
public void Domain_FirstNameValue_ValueObjectCreated()
{
    //Arrange
    var PersonFirstName1 = new PersonFirstName("Ian");
    var PersonFirstName2 = new PersonFirstName("Ian");
    //Act
    var firstNameCompareResult = PersonFirstName1.Equals(PersonFirstName2);
    //Assert
    Assert.True(firstNameCompareResult);
}

I would be grateful for comments on the quality of this test.

A few notes:

The argument exceptions contain an incorrect parameter name ("Surname"). Use nameof(firstName) instead: this allows the compiler to verify that you're referencing an existing identifier, and the refactoring tools of your IDE can automatically update it when you rename that parameter.
Why are you setting both this._firstName and FirstName in the constructor? That's doing double work.
A read-only automatic property seems appropriate here: public string FirstName { get; }. This can only be assigned within a constructor, similar to readonly fields, and doesn't require you to explicitly create a backing field.
I see a disabled property with a public setter. If it wasn't disabled, then FirstName instances could not guarantee that their GetHashCode would always return the same value. That could lead to subtle bugs such as failing dictionary lookups, so you did well to make your class immutable.
You may want to implement IEquatable<T>. It's not a replacement for Equals and GetHashCode, but it lets you provide a strongly-typed equality check, which can prevent unnecessary type-checks or boxing/unboxing, and it clearly signals intent. Likewise, if you need to order or sort these first-names, take a look at IComparable and IComparable<T>.
Why does IsValid not allow names longer than 255 characters? If there's a good reason for that, then why isn't it documented? And does that restriction always apply, or only in a certain context? And is a simple false or ArgumentException sufficient for the caller, or do they need to distinguish between different validation failures?
The test method is incorrectly named: according to the name, it's testing object creation, but it's actually testing equality. Domain_EqualFirstNameValues_Equals would be more accurate.
Assert.True has an overload that accepts a failure message. A good message (perhaps including the values that are involved) can make it easier to determine why a test is failing.


Base64Encoder class wrapper I'm new in python and I'm wondering if there any issue I should worry about in the following code?

from base64 import b64encode

class Base64Encoder:

    @classmethod
    def encode_basic(cls, usr, pwd):
       token = cls.base64_encode(f"{usr}:{pwd}")
       return f'Basic {token}'

   @classmethod
   def encode_bearer(cls, token):
       return f'Bearer {token}'

   @staticmethod
   def base64_encode(token):
       return b64encode(token.encode('ascii')).decode('ascii')

You've got a class with only two classmethods and one staticmethod. One classmethod does not use the class argument, the other one refers to a static method. Hence none of the class' methods use either the class or one of its instances. Thus, you do not require a class at all:

def encode_basic(usr, pwd):
    token = base64_encode(f"{usr}:{pwd}")
    return f'Basic {token}'

def encode_bearer(token):
    return f'Bearer {token}'

def base64_encode(token):
    return b64encode(token.encode('ascii')).decode('ascii')

Also consider adding docstrings to the functions, that explain how they are supposed to be used. Without knowlege of your program's context it is hard to tell, why the strings are being formatted in the respective ways and why one string needs ASCII encoding.


Life, the Universe, and Everything Your program is to use the brute-force approach in order to find the Answer to  Life, the Universe, and Everything. More precisely... rewrite small numbers from input to output. Stop processing input after reading in the number 42. All
  numbers at input are integers of one or two digits.

I submitted the code for the above problem on spoj and it says wrong answer even when writing the correct output.

The first solution I submitted was:

#include <stdio.h>
int main(void)
{
    int i;
    while (1) {
        scanf("%d", &i);
        if (i == 42)
            break;
        printf("%d", i);
    }
    return 0;
}

The above solution doesn't check whether the input is two digit or not so I wrote another one:

#include <stdio.h>
int main(void)
{
    int i;
    while (1) {
        scanf("%d", &i);
        if (i > 99)
            continue;
        if (i == 42)
            break;
        printf("%d", i);
    }
    return 0;
}

  ... rewrite small numbers from input to output. All numbers at input ... one or two digits.

However, both of the above solutions don't work on spoj.

Disclaimer: I'm not sure if this code will be the solution for SPOJ.

You don't check if the input number is an int.

if (scanf("%d", &i) == 1)
{
    printf("OK\n");
}
else
{
    printf("Not an integer.\n");
}

You don't ever tell the user what to enter.

printf("%s\n", "Enter what you think the answer is to life, the universe, and everything (hint: it's a number).");

This is more of a nitpick, but the while loop could be created in a better fashion instead of breaking.

while (i != 42)

Another nitpick, you don't add a newline after printing the int.

printf("%d\n", i);


Generating Passwords with a Secure PRNG What's the best way to approach generating a password using a secure prng? In Python, I could simply use os.urandom. Any suggestions on this?

 _ = require('underscore')

exports.gen = (n=10) ->
  throw new Error 'Not a number!' if typeof(n) isnt 'number'
  throw new Error 'Prefered (n): [10..128]' if n < 10 or n > 128
  chars = (String.fromCharCode(i) for i in [33..126]).join ''
  _.times(n, -> chars[(Math.random() * chars.length) | 0]).join ''

The equivalent to os.urandom is found in the crypto module: crypto.randomBytes. That module also contains most other functions you might want for password hashing etc.

To use the same password character set as your current code, you could do

exports.gen = (length = 10) ->
  throw new Error 'Length is not a number!' if typeof length isnt 'number'
  throw new Error 'Length must be [10..128]' unless 10 <= length <= 128

  range = 126 - 33
  buffer = require('crypto').randomBytes length # note: may throw an error

  (String.fromCharCode(33 + (range * c / 255) | 0) for c in buffer).join ''

I've changed a few small things

length instead of n
Slightly more descriptive error messages, i.e. what is not a number, and sterner wording for bad length values ("prefered" [sic] makes it sound as though it's just a suggestion, but it's actually a requirement)
CoffeeScript has chained comparisons like Python, so 10 <= length <= 128 can be used


String to Integer (atoi) I was doing following question on leetcode: String to Integer (atoi)

Question:

  Implement atoi which converts a string to an integer.

  The function first discards as many whitespace characters as necessary
  until the first non-whitespace character is found. Then, starting from
  this character, takes an optional initial plus or minus sign followed
  by as many numerical digits as possible, and interprets them as a
  numerical value.

  The string can contain additional characters after those that form the
  integral number, which are ignored and have no effect on the behavior
  of this function.

  If the first sequence of non-whitespace characters in str is not a
  valid integral number, or if no such sequence exists because either
  str is empty or it contains only whitespace characters, no conversion
  is performed.

  If no valid conversion could be performed, a zero value is returned.

  Note:

  Only the space character ' ' is considered as whitespace character.
  Assume we are dealing with an environment which could only store
  integers within the 32-bit signed integer range: [−231,  231 − 1]. If
  the numerical value is out of the range of representable values,
  INT_MAX (231 − 1) or INT_MIN (−231) is returned.

Question link: https://leetcode.com/problems/string-to-integer-atoi/

For the above question, I wrote the following code

/**
 * @param {string} str
 * @return {number}
 */
var myAtoi = function(str) {
    let i = 0
    let output = ''
    const intMax = 2**31 - 1
    const intMin = -intMax - 1
    while (i<str.length) {
       const char = str[i]
       if (!char == " ") {
           if (char.toLowerCase() === char.toUpperCase()) {
               output = output + char
           } else if (output) {
               break
           }
           if (!output) return 0
       }
        i++
    }
    output = parseInt(output)
    if (output !== null && !isNaN(output)) {
        if (output > intMax) return  intMax
        if (output < intMin) return intMin
        return output
    }
    return 0
}

Can someone improve upon the same and give me suggestions on how I can improve my code?

Unfortunately there are several problems with your code. It only reason it works is because you are using parseInt which (as @wOxxOm shows) fulfills the original task requirements and thus ignores your broken attempts at cleaning the string.

!char == " "

This expression always returns true. Like in most programming languages negation (!) have a higher precedence than the comparison ==, so !char is resolved to false (unless char were an empty string, which in this case it never is) and false == " " resolves to true (to be honest I'm not sure why, I'd have to look it up. JavaScript's rules for non-strict comparison (==) are quite complex.)

So it should be char != " " , or even better char !== " ". Always use strict comparison, unless you have a good reason not to.

char.toLowerCase() === char.toUpperCase()

This is a strange attempt to identify digits (and +/-). There are countless other characters that are not filtered by this.

32-bit signed integer range

You are missing the point of this rule. When the system is assumed not to be able to store integers outside the range, then that means a variable can't hold such a large/low number, so a comparison such as output > intMax can never be true, since output can't physically contain a number larger than intMax.

Don't reuse variables

In output = parseInt(output) you change the type and the semantics of the variable output. This is confusing for the reader. And the name output for the original use of collecting the validated string wrong anyway.

Don't leave out semicolons

JavaScript's automatic semicolon insertion is nearly as complex as its non-strict comparison rules. By leaving out the semicolons you make the code more difficult to read because the reader then has an additional thing they need to consider.

Finally

The big problem here is you using parseInt. For one, as mentioned it's hiding the bugs in the code, and more importantly, you are missing the whole point of this exercise, which is to understand how string to number conversion actually works.


Locking during cache population Here I want to lock when populating a particular cache object, without blocking other calls to the same method requesting Foos for other barIds. I realise the MemoryCache will be thread safe, but if two or more concurrent calls come in it seems like it would be better if only one of them populates the cache and locks the others out while this is done.

using System.Runtime.Caching;
using Microsoft.Practices.Unity;

public class FooCacheService : IFooService
{
        private static readonly ConcurrentDictionary<string, object> FooServiceCacheLocks = new ConcurrentDictionary<string, object>();

        [Dependency("Explicit")]
        public IFooService ExplicitService { get; set; }

        public IEnumerable<Foo> GetFoosForBar(int barId)
        {
            string key = barId.ToString();

            object lockObject = FooServiceCacheLocks.GetOrAdd(key, new object());

            object cached = MemoryCache.Default[key];

            if (cached == null)
            {
                lock (lockObject)
                {
                    cached = MemoryCache.Default[key];

                    if (cached == null)
                    {
                        IEnumerable<Foo> foosForBar = this.ExplicitService.GetFoosForBar(barId);

                        MemoryCache.Default.Add(key, foosForBar, DateTime.Now.AddMinutes(5));

                        return foosForBar;
                    }
                }
            }

            return (IEnumerable<Foo>)cached;
        }
}

Assume GetFoosForBar is a CPU/IO intensive process and takes a short while to complete. Does this seem like a reasonable way to achieve this? I'm surprised that System.Runtime.Caching does not provide any support for this as default - or have I overlooked something?

What I don't like about your approach is that the lock objects are never removed from FooServiceCacheLocks, even when the object is removed from the cache.

One way to simplify your code would be to combine MemoryCache with Lazy: instances of Lazy are cheap (as long as you don't access its Value) so you can create more of them than needed. With that your code would look something like this:

public IEnumerable<Foo> GetFoosForBar(int barId)
{
    var newLazy = new Lazy<IEnumerable<Foo>>(
        () => this.ExplicitService.GetFoosForBar(barId));

    var lazyFromCache = (Lazy<IEnumerable<Foo>>)MemoryCache.Default.AddOrGetExisting(
        barId.ToString(), newLazy, DateTime.Now.AddMinutes(5));

    return (lazyFromCache ?? newLazy).Value;
}


Function to lock a file using memcache, Version 1 I have a web application where I list files in the UI to many users at the same time. There are many people who monitor the files and process them. However, I don't want multiple users processing the same file.

I am using simple memcache dictionary as data structure to interpret if it is locked. If the key "filename" is present then it is considered as locked, otherwise the key "filename" is added (maybe with username as value) so that another user cant access it and access is granted for that user. The files "locked" by a user is processed at a later point by that user itself and no other. "Process" here means to move it to some another folder.

I was recommended to use Ghetto Central Locking by the development team but I still feel that it doesn't cover my scenario of locking and delayed action. The  problem there is that the "locking" and "action" both are combined operation but in my case can be delayed. Also the user should have the ability to unlock and not specify a time-out value as mentioned there.

So I came up with an API that covers my scenarios:

def addToCart(filename):
    ableToLock = memcache.add(filename, username)
    fileExists = os.file.ispath(filename)
    if ableToLock:
        if fileExists:
            return "Added To Cart"
        else:
            memcache.delete(filename)
            return "Processed by another user."
    else:
        if fileExists:
            return "Processed by another user."
        else:
            user = memcache.get(filename)
            return "%s is Processing" % user

def process(source):
    shutil.move(source, destination)

process happens any point of time later.

Does anyone see logical flaws or race-conditions if this code is run concurrently?

I consider it as flawed if a request comes from a user to process one of his locked file and one of the ever gets a file not found error. Scenarios like DOM manipulation etc are not considered valid because it interferes with the logic.

PS: Memcache has some drawbacks if we use as a locking mechanism like for example remote server disconnection and data eviction, but we don't consider that as an issue.

The code only acquires a lock for the source of the move, and not for the destination. This means that there's a race between two users trying to move two different files to the same destination.
There's a race between:

memcache.delete(filename)

and:

user = memcache.get(filename)

It's not clear how the lock is released. It doesn't happen in process.
The test else: if fileExists: seems backwards.


My implementation of the repository pattern I'm using this pattern since a few months and I was wondering if I can make it any better.

The one thing I am not satisfied about is the dispose method. In every repository I have to add a dispose method that calls the base dispose method. Is there a way to get rid of the dispose method in every repository and just let it (automatically) call the base dispose method?

I'm using Linq2Sql.

Interface:

  public interface IRepository<TEntityType> where TEntityType : class
{
    TEntityType GetById(int id);

    IQueryable<TEntityType> GetAll();

    void Delete(TEntityType item);

    void Add(TEntityType item);
}

Base:

  public class BaseRepository<TEntityType> where TEntityType : class
{
    private readonly DataClassesDataContext _dataContext;

    protected BaseRepository()
    {
        _dataContext = new DataClassesDataContext();
    }

    protected Table<TEntityType> GetTable()
    {
        return _dataContext.GetTable<TEntityType>();
    }

    protected void SaveChanges()
    {
        _dataContext.SubmitChanges();
    }

    protected virtual void Dispose()
    {
        if (_dataContext != null)
        {
            _dataContext.Dispose();
        }
    }
}

Repository class:

  public class FirstClassRepository : BaseRepository<FirstTable>, IRepository<FirstTable>, IDisposable
{
    public FirstTable GetById(int id)
    {
        return GetTable().FirstOrDefault(x => x.Test == id);
    }

    public IQueryable<FirstTable> GetAll()
    {
        return GetTable();
    }

    public void Delete(FirstTable item)
    {
        GetTable().DeleteOnSubmit(item);
    }

    public void Add(FirstTable item)
    {
        GetTable().InsertOnSubmit(item);
    }

    public new void Dispose()
    {
        base.Dispose();
    }
}

The one thing I am not satisfied about is the dispose method. In every repository I have to add a dispose method that calls the base dispose method.

IDisposable
The IDisposable interface should be impelemented in the base class. By using the default pattern of disposing, you will get this result

public class BaseRepository<TEntityType> : IDisposable where TEntityType : class
{
    private readonly DataClassesDataContext _dataContext;

    protected BaseRepository()
    {
        _dataContext = new DataClassesDataContext();
    }

    protected Table<TEntityType> GetTable()
    {
        return _dataContext.GetTable<TEntityType>();
    }

    protected void SaveChanges()
    {
        _dataContext.SubmitChanges();
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~BaseRepository()
    {
        Dispose(false);
    }

    protected void Dispose(Boolean disposing)
    {
        // free unmanaged ressources here
        if (disposing)
        {
            // This method is called from Dispose() so it is safe to
            // free managed ressources here
            if (_dataContext != null)
            {
                _dataContext.Dispose();
            }
        }
    }
}

But wait, what if you also need to dispose ressources of the FirstClassRepository object which descends from that object. Then the FirstClassRepositoryobject also needs to implement the IDisposable interface with the pattern below.

public class FirstClassRepository : BaseRepository<FirstTable>, IRepository<FirstTable>, IDisposable
{
    public FirstTable GetById(int id)
    {
        return GetTable().FirstOrDefault(x => x.Test == id);
    }

    public IQueryable<FirstTable> GetAll()
    {
        return GetTable();
    }

    public void Delete(FirstTable item)
    {
        GetTable().DeleteOnSubmit(item);
    }

    public void Add(FirstTable item)
    {
        GetTable().InsertOnSubmit(item);
    }

    public void Dispose()
    {
        try
        {
            Dispose(true); //true: safe to free managed resources
            GC.SuppressFinalize(this);
        }
        finally
        {
            base.Dispose();
        }
    }

    ~FirstClassRepository()
    {
        Dispose(false);
    }

    protected void Dispose(Boolean disposing)
    {
        // free unmanaged ressources here
        if (disposing)
        {
            // This method is called from Dispose() so it is safe to
            // free managed ressources here
        }
    }
}

If the FirstClassRepository obect doesn't need to dispose ressources you can omit the implementation of IDisposable.

EDIT based on comment:

The ~FirstClassRepository() method is what the compiler takes as beeing the Finalize() method of the object. If you forget to call the Dispose() method of that object, this method will be called at the time your object is destructed.
So as the method is implemented by calling the overloaded Dispose(Boolean) method with the parameter == false, only unmanaged ressources will be freed (where the comment is written) as the GC takes care of the managed objects.

Otherwise your implementation looks fine for me.

Some SO links
proper-use-of-the-idisposable-interface
when-should-i-use-gc-suppressfinalize


Stack implementation using vectors with no overflow version 1.0 This is an implementation of a stack where I have used vectors. Can anyone please review it and let me know how I can improve this code and overall coding practice?

    #include"iostream"
class Mystack
{
private:
    int *A;
    int top;
    int size;
public:
    Mystack(int capacity);
    void push(int x);
    int pop();
    int topElement();
    void print();
    bool isEmpty();
};
Mystack::Mystack(int capacity)
{
    top = -1;
    size = capacity;
    A = new int[capacity];
}
void Mystack::push(int x)
{

    if (top + 1 == size)
    {
        int *vec = new int[size + size];
        for (int i = 0; i < size; i++)
        {
            vec[i] = A[i];
        }
        A = vec;
        size = size * 2;
        top++;
        A[top] = x;
        return;
    }
    else
    {
        top++;
        A[top] = x;
    }
}
int Mystack::pop()
{
    if (isEmpty())
    {
        std::cout << "Stack Underflow" << std::endl;
    }
    else
    {
        std::cout << "The popped element is" << A[top];
        return A[top--];
    }
    return 0;
}
bool Mystack::isEmpty()
{
    if (top == -1)
    {
        std::cout << "Is Empty" << std::endl;
        return true;
    }
    else
    {
        std::cout << "Not Empty" << std::endl;
        return false;
    }
}
int Mystack::topElement()
{
    std::cout << "The top element is : " << A[top];
    return A[top];
}
void Mystack::print()
{
    for (int i = 0; i <= top; i++)
    {
        std::cout << A[i] << " ";
    }
}
void main()
{
    Mystack s1(10);
    int num, ch = 1;
    while (ch >0)
    {
        std::cout << "\n1. PUSH" << std::endl;
        std::cout << "2. TOP" << std::endl;
        std::cout << "3. IsEmpty" << std::endl;
        std::cout << "4. POP" << std::endl;
        std::cout << "5. EXIT" << std::endl;
        std::cout << "6. Print" << std::endl;
        std::cout << "Enter the choice" << std::endl;
        std::cin >> ch;
        switch (ch)
        {
        case 1:
            std::cout << "Enter the number to be pushed" << std::endl;
            std::cin >> num;
            s1.push(num);
            break;
        case 2:
            std::cout << "Get the TOP Element" << std::endl;
            s1.topElement();
            break;
        case 3:
            std::cout << "Check Empty" << std::endl;
            s1.isEmpty();
            break;
        case 4:
            std::cout << "POP the element" << std::endl;
            s1.pop();
            break;
        case 5: exit(0);
        case 6:
            s1.print();
            break;
        }
    }
    system("pause");
}

What is A?  You shouldn't use single-character variables as they're very ambiguous.  You may know what it's for, but it doesn't mean anyone else does.
Any member function that doesn't modify any data members should be const.

One example is topElement():

int Mystack::topElement() const
{
    std::cout << "The top element is : " << A[top];
    return A[top];
}

This class performs memory allocation, but you never deallocate it, making it one big memory leak for anyone using it.

You should have a destructor that uses delete to free this memory:

Mystack::~Mystack()
{
    delete[] A;
}

With a destructor in place, you should also have a copy constructor and assignment operator defined, as per the Rule of Three.  It is especially important to define your own since the default ones perform a shallow copy, which shouldn't be done when you have pointers as data members. Those will instead be copied instead of the data they point to, which will mess up your data.
Although pop() "reports" underflows, it can still break since it doesn't handle errors properly.  Specifically, it'll return 0 if the stack is already empty.  Since 0 is a value, the calling code can store it like normal.  So, pop() can be called repeatedly and it will always return this value.

What you should do is have the function return void so that it won't have to return anything if the stack is empty.  This is also how it's done in the standard library, in case you weren't aware.
Something like this shouldn't be displaying simple output, though it may be okay if you're just testing the functions.  Otherwise, it should be done in main(), while the bool values should determine which messages should be displayed.
One case where output is really not needed is in isEmpty().  A simple function like this can just be done in one line:

return (top == -1);

It's important to know that main() should return int.  Some compilers allow it to return void, but this isn't allowed by the standard.  Regardless, you should change it to return int.
Be aware that system("PAUSE") is Windows-specific thus non-portable.  If you'd like to do a similar pause that is portable, use something like std::cin.get().
If you'd like for this stack class to handle types other than int, then consider using templates.  There are many resources on this, and it should be worthwhile to learn about this.


Shorten my asynchronous DB queries, using async.js module I am using

Node.js
Express.js
node-mysql.js
async.js

I am making multiple asynchronous queries to my DB, but every query relies on the previous one's results, so I decided to use the waterfall method.

Right now, my code looks like this (I simplified it with only 2 queries, but there are more):

async.waterfall([
    function(cb){
        db.query(insertQuery,values,cb);
    },
    function(results,_,cb){
       var lastInsertId = results.insertId;
       db.query(anotherInsertQuery,otherValues.concat(lastInsertId),cb);
    }
],callback);

But I found my code a bit messy, especially the function(cb) { ... } wraps.

Is there a way to get rid of those annoying function(cb){...} ?

I think not.

Whereas you find the function(cb) { ... } wraps a bit messy, I think that this is the most elegant way to show readers that this is a separate functions, and that stuff is about to get asynchronous.. Again, compared to other approaches, this is quite clean.

Also, (I know this is just a small example), consider not using the lastInsertId variable. You could simply go for

async.waterfall([
    function(cb){
        db.query(insertQuery,values,cb);
    },
    function(results,_,cb){
       db.query(anotherInsertQuery,otherValues.concat(results.insertId),cb);
    }
],callback);


Searching for an object in a list returning boolean and referencing variable My method searches through an list of accounts which have the property "Name".

class Accounts
{
    static internal List<Account> accounts = new List<Account>();

    static internal bool findAccount(string name, ref int account)
    {
        for (int i = 0; i < accounts.Count; i++)
        {
            if (accounts[i].Name == name)
            {
                account = i;
                return true;
            }
        }
        return false;
    }
}

The variable it is referencing is in the main form:

public partial class Lodge : Form
{
    public Lodge()
    {
        InitializeComponent();
    }

    private void btn_lodge_Click(object sender, EventArgs e)
    {
        int account = -1;
        bool found = Accounts.findAccount(tBox_wName.Text, ref account);
        if (found && account >= 0)
        {
            decimal lodge = Convert.ToDecimal(tBox_tAmou.Text);
            Accounts.accounts[account].lodge(lodge);
            lbl_stats.Text = "Successfully lodged. Balance is now: " + Accounts.accounts[account].Balance;
        }
        else
        {
            lbl_stats.Text = "Account not found.";
        }
    }

I am still learning C#, so I am still learning the most efficient ways of doing things.  This code works as it should, but I feel like the fact that I am returning a boolean and referencing the account variable seems wrong.

The program is basically lodging money to an account (simply an object containing the person's name, etc..).  So there is a static Accounts class and a non-static class 'Account' for each account.  I have made a static class because these functions are to be shared across multiple forms.

Should I remove the boolean and simply have it return the account to the account variable?  What would be the most accepted way by a more experience programmer?

The List type provides a List-specific Exists method which will return a boolean telling you whether a value that meets the specified predicate exists in the list:

var found = Accounts.Exists(a => a.Name == tBox_wName.Text);

Instead of using that though, just use Find(predicate) to return the first value for which the predicate returns true (or the type's default value if not found which is null for reference types).

Array has similar methods.

If you're using any IEnumerable you can use .Any() or FirstOrDefault().

If I was rolling this by hand (which in your case I don't believe is necessary), I'd prefer the following pattern:

Account account;
if(TryFindAccount(accounts, out account))
{
    ....
}

or

Account account;
if(TryFindAccountByName(accounts, name, out account))
{
    ....
}

Unless you really needed to track by index across layers (for which I don't usually find a compelling reason to do so), I'd deal directly with the account as soon as possible.

The above implementation also avoids checks against magic values, like checking if account is null, or in your example, -1. If you can, avoid attributing special meaning to certain values.


Find the highest product between elements in a set I'm solving a programming challenge that essentially consists of finding the highest product of elements in a set.

This is my current solution, and is passing all the test cases but one. The test case that is failing is not available to me, so I kinda have to guess what is wrong with my code.

public static String solution(int[] xs) {
    if (xs.length == 1) {
        return Integer.toString(xs[0]);
    }

    Arrays.sort(xs);

    BigInteger productOfPositiveIntegers = BigInteger.ONE;
    BigInteger productOfNegativeIntegers = BigInteger.ONE;
    Vector<BigInteger> negativeIntegers = new Vector<>();
    BigInteger result;

    boolean areAllElementsZero = true;

    for (int element : xs) {
        if (element > 0) {
            productOfPositiveIntegers = productOfPositiveIntegers.multiply(new BigInteger(Integer.toString(element)));
            areAllElementsZero = false;
        } else if (element < 0) {
            negativeIntegers.add(new BigInteger(Integer.toString(element)));
            areAllElementsZero = false;
        }
    }

    if (areAllElementsZero) {
        return BigInteger.ZERO.toString();
    }

    if (negativeIntegers.size() % 2 == 0) {
        for (BigInteger element: negativeIntegers) {
            productOfNegativeIntegers = productOfNegativeIntegers.multiply(element);
        }
    } else {
        negativeIntegers.remove(negativeIntegers.size() - 1);
        for (BigInteger element: negativeIntegers) {
            productOfNegativeIntegers = productOfNegativeIntegers.multiply(element);
        }
    }

    if (productOfNegativeIntegers.compareTo(BigInteger.ZERO) == 1) {
        result = productOfPositiveIntegers.multiply(productOfNegativeIntegers);
    } else {
        result = productOfPositiveIntegers;
    }

    return result.toString();
}

My algorithm is essentially:

sort numbers from lowest to highest.
keep track negative numbers in a separate vector.
multiply all positive numbers
If the total number of negative numbers is even, multiply them all. If not, drop the lowest negative number and multiply all the negative ones.
If the product of all the negative numbers is bigger than zero, multiply the product of negative numbers with the product of all positive numbers.
Return solution.

These are the test cases that I'm passing to my method:

    int[] testCase = {2, 0, 2, 2, 0}; // should return "8"
    int[] testCase2 = {-2, -3, 4, -5}; // should return "60"
    int[] testCase3 = {2, -3, 1, 0, -5}; // should return "30"
    int[] testCase4 = {-4, -10, -2, -1, -5}; // should return "400"
    int[] testCase5 = {1, 2, 3, 4};
    int[] testCase6 = {0, 0, 0}; // should return "0"
    int[] testCase7 = {-1}; // should return "-1"
    int[] testCase8 = {-1, -1, -1}; // should return "1"

For example the first test case, should return 8 because the highest product would be:

2 * 2 * 2 = 8

Second test case is 60 because:

-4 * -5 * 4 = 60

Last test case is 1 because:

-1 * -1 = 1

My assumption is that my code is taking too long to run for the test case that is failing.

According to the challenge, the highest possible number is 1000, and the highest number of elements in the set would be 50, so the biggest product would be 1E150 When I pass that test case to my program, it runs in less than a second.

So my question is two parts really:

What can I do to speed this up? I'm casting integers to BigIntegers in a loop, so that hurts just to look at it. But I kinda need to do that anyways.
If performance is not the problem, and I'm missing one edge case, which one is it?

Also, if this is not a good question for this community please let me know and I'll be more than happy to remove the question.

Test case that could be failing: {0,-1}.
In your case it returns 1 while it should have been 0.

About the algorithm:

Since you are already sorting it, just multiply everything except zero and first negative number. If its still negative multiply the first negative number.


Saving array values for a secondary foreach loop Although used within the WordPress platform, this is PHP focussed question.

I have an array of child posts, some have content, some don't. My client wishes to display the posts in alphabetical order, but posts without content are to be positioned at the end (still in A-Z order).

My solution is to assign posts that lack content to an array, use continue to move the main array counter forward and then foreach over the the new array at the end:

<?php

    $child_pages = pp_get_child_pages();

    if ( count( $child_pages ) != 0 ) :

        $empty_child_pages = array();

        echo '
';

        foreach ( $child_pages as $post ) :

            setup_postdata( $post );

            if ( $post->post_content == "" ) {
                $empty_child_pages[] = $post;
                continue;
            }

            pp_get_template_part( 'content', 'child-page' );

        endforeach;

        foreach ( $empty_child_pages as $post ) :

            setup_postdata( $post );

            pp_get_template_part( 'content', 'child-page' );

        endforeach;

        echo '
';

    endif;

    wp_reset_postdata();

?>

Is there a more efficient way to write this?

Would you modify the array during the initial loop?

Your approach seems fine, assuming that the entries in $child_pages are the correct order. I mention that because you mentioned the ordering requirement in the description, but I don't see any supporting logic in the posted code.

If you need to sort the entries yourself, another solution can be sorting using usort() and a custom comparator function:

if the second post is empty, then the first is smaller
else if the first part is empty, then the second is smaller
else perform regular alphabetic comparison

Using such comparator, empty posts will get sorted to the end.


Sample n items without replacement in Prolog I have the following Prolog code which samples n items from a list without replacement:

samp(0,_,Sample,Sample).
samp(N,Domain0, Sample0, Sample):-
  N >= 0,
  length(Domain0,L),
  RandomNumber is random(L),
  nth0(RandomNumber, Domain0, X),
  delete(Domain0,X,Domain1),
  append(Sample0,[X],Sample1),
  N1 is N-1,
  samp(N1,Domain1,Sample1,Sample).

sample(Domain,N,Sample):-
  samp(N,Domain,[],Sample).

Is there a 'nicer' way to do this?

In SWI-Prolog you could do this that way:

sample_swi(Domain, N, Sample) :-
    random_permutation(Domain, Permutation),
    length(Sample, N),
    append(Sample, _, Permutation).

Be careful, this predicate is not a true relation (just as in your code).


Applicative permutation to generate knight moves I want to understand Haskell better by trying to stretch possibilities of Functor, Applicative, and Monad as much as possible and study how they behave. So in Learn You a Haskell there's an exercise to generate all possible moves for a knight from a given position on chessboard:

moveKnight :: KnightPos -> [KnightPos]
moveKnight (c,r) = filter onBoard
    [(c+2,r-1),(c+2,r+1),(c-2,r-1),(c-2,r+1)
    ,(c+1,r-2),(c+1,r+2),(c-1,r-2),(c-1,r+2)
    ]
    where onBoard (c,r) = c `elem` [1..8] && r `elem` [1..8]

Now I thought to myself, wait a second, I can actually generate that list [(c+2,r-1),(c+2,r+1),(c-2,r-1),(c-2,r+1),(c+1,r-2),(c+1,r+2),(c-1,r-2),(c-1,r+2)] instead of hardcoding it. So this is how I did it with list comprehensions:

invert x
  | x == 1 = 2
  | x == 2 = 1
  | otherwise = 0

moves = [(c `f` a, r `g` (invert a)) |
         c <- [6],
         r <- [2],
         f <- [(+), (-)],
         g <- [(+), (-)],
         a <- [1, 2]]

-- generates: [(7,4),(8,3),(7,0),(8,1),(5,4),(4,3),(5,0),(4,1)]

So the above works correctly, but the code looks ugly. Because every list comprehension is equivalent to a lift, the above code could be rewritten with liftA5:

liftA5 :: Applicative g => (a -> b -> c -> d -> e -> f) -> g a -> g b -> g c -> g d -> g e -> g f
liftA5 f a b c d e = f <$> a <*> b <*> c <*> d <*> e

liftA5 (\c r f g a -> (c `f` a, r `g` (invert a))) [6] [2] [(+), (-)] [(+), (-)] [1,2]

My questions are:

Are there any way to generate the above list in a more elegant way?
How to generalize it to arbitrary possible permutations? (So that my code could parametrized and not fixed to just numbers 1 and 2, or just + and -, etc)
What intuitions, insights and lessons can I get from this exercise?

I know that this is a massively contrived situation, but they serve me as an exercise to understand the language better.

Using pattern matching you can write:

invert 1 = 2
invert 2 = 1
invert n = 0 -- Error would be better in my opinion

You can inline 6 and 2 instead of taking them from a single item list.


File Exists Script Task for SQL Server SSIS I have an SSIS package for SQL Server 2008. In this package, I need to verify that a file exists that matches a pattern of SomeFileName_*.txt where the wild card replaces a YYYYMMDD date. It doesn't matter what the date of the file is, just that any file matching that pattern exists. If a file does not exist, I need to fail the package before using the built in "For each file loop" task. This allows our server to alert me to the package failure.

It's a pretty simple bit of code, but it has a huge smell. I'm catching Exception. I think I'm doing it for the right reasons though. Basically, I'm saying that if any exception gets thrown, fail the script task, and thus the package. Is this a case where it's okay to catch all exceptions??? For reference, these are the possible exceptions that Directory.GetFiles can throw.

Of course, I'm pretty new to c#, so please let me know if I'm doing anything stupid. (Yes, I know I'm programming in Main, but this is short and sweet. I don't see the harm just this once...)

public void Main()
{

    try
    {
        string sourcePath = Dts.Variables["User::sourcePath"].Value.ToString();
        string searchPattern = Dts.Variables["User::searchPattern"].Value.ToString();

        string[] files = Directory.GetFiles(sourcePath, searchPattern, SearchOption.TopDirectoryOnly);
        if (files.Length > 0)
        {
            Dts.TaskResult = (int)ScriptResults.Success;
        }
        else
        {
            Dts.TaskResult = (int)ScriptResults.Failure;
            Dts.Log(string.Format("No file(s) found in '{0}' with a pattern of '{1}'.", sourcePath, searchPattern), 0, null);
        }
    }
    catch (Exception e)
    {
        Dts.TaskResult = (int)ScriptResults.Failure;
        Dts.Log(string.Format("Script Task encountered an unexpected error. Souce: {0}. Message: {1).", e.Source, e.Message), 0, null);
    }
}

Yes, it's okay. If you need to catch any exception then you should essentially catch them all.

The reason you typically avoid this and use the most concrete exceptions possible is to provide exception-specific behaviour: an accurate log message or a different continuation once it has been logged.

If all your exceptions are handled the same way then you can just use the catch-all. Note that you can always add a specific handler afterwards because the catch clauses are evaluated linearly.


Batch file to zip logs date wise This is the batch file I have created which zips the particular month logs and deletes those logs from the source folder after successful zipping. If it does not find the zip file, then it again starts to zip the logs.

Please suggest any changes to be done.

@echo off

set zip="C:\Program Files\7-Zip\7z.exe"
rem Findout Month, Year and Date
FOR /F "tokens=1,2,3,4 delims=/ " %%A in ('Date /t') do set year=%%D
FOR /F "tokens=1,2,3,4 delims=/ " %%A in ('Date /t') do set month=%%B
FOR /F "tokens=1,2,3,4 delims=/ " %%A in ('Date /t') do set day=%%C

rem Solve problem Year start and adding 0 in month
if %month% EQU 01 (
    set month=12
    set /a year=%year%-1
) else (
    set /a month=%month%-1
)
if %month% LSS 10 set month=0%month%

rem Set file names for last month file.
set lastmonthfiles=server.log.%year%-%month%-
rem To compress the file.
:compress
%zip% -tzip  a -y "bkp-%lastmonthfiles%.zip" %lastmonthfiles%*
ping -n 5 192.168.100.44 > nul
if Not exist bkp-%lastmonthfiles%.zip (
    echo zipping failed
    pause
) else (
 DEL %lastmonthfiles%*

 )
pause

Well, first off, I must commend you on the readability of your code.  You've employed nice spacing and enough comments to be able to find different sections easily without a lot of reading.  I think I could learn a thing or two from you here.  Nice job!

There are a few things you can do to improve your code, though.  Let's start with the general practices first.

General practice advice

Unless your script is intended to set environment variables for other scripts or applications, you should always use setlocal.  Even if the script you're writing is intended to append a new directory to %PATH%, you should still setlocal at the top until your internal flow is complete and you're ready to commit the change to %PATH%.  This way you don't pollute your environment with a bunch of variables that only have meaning within a particular script -- or worse, have meaning in a different script that expects the variable not to be defined yet.  Whenever you @echo off, setlocal should automatically be the next thing you type.
When setting variables to string values, it's good practice to set "varname=string" with the quotation marks surrounding both the variable and its value.  That way, whenever you use your variable later, there's no ambiguity whether your variable="value" or variable=value.  Also, in a future script, you might capture special characters into a variable, like an ampersand or a percent.  Get into the habit of set "variable=value" now and you won't have to change your coding style for special cases like that, and you'll spend less time debugging.
Likewise, in your if statements, you should enclose the items on both sides of your comparison operator.  if "%foo%" equ "%bar%", or if "%%~xI"==".exe".  I can't count all the times as a rookie scripter I would struggle with errors when %foo% contained a space, causing "blah was unexpected at this time" because I didn't use quotes.
set /a has some shorthand syntax you might find helpful.  Interestingly, when you're doing math with variables, you don't have to use % around the variable names.  For example, instead of set /a year=%year%-1 you can set /a year=year-1.  You can also combine operator and assignment like +=, *=, /=, etc.  So instead of set /a year=%year%-1 you can set /a year-=1.

Now, there are a few issues specific to this script that can be improved.

Script-specific suggestions

%date% and date /t are ambiguous.  Some locales list date as MM/DD/YYYY, while others use DD/MM/YYYY, and still others use YYYY/MM/DD.  (more information.)  A more agnostic way of scraping the date would be to use wmic.  See Method 2 on this page for a way to put the date into variables that should work more universally.
When using del with a wildcard, consider adding the /q switch to suppress confirmation, unless you intentionally want your script to ask the user to confirm deletion.
Consider changing ping -n 5 192.168.100.44 > nul to ping -n 5 0.0.0.0 >NUL.  Having an actual IP there might (at at glance) prompt the reader to wonder whether the script will behave differently whether the host does or does not respond; whereas 0.0.0.0 makes it obvious that you're using the ping command as nothing more than a period of sleep.
if Not exist bkp-%lastmonthfiles%.zip <-- If this is ever true, you are going to pause twice, then exit.  Examine your logic here.  Did you leave out a goto compress?

What it looks like you intended to do is attempt to zip; then if the zip file doesn't exist, echo a notice to the user, pause, then loop back to :compress to try again.  Otherwise, assume everything went fine and delete all the old stuff and exit.  What happens if a file is in use and locked, and 7-zip skips archiving it but was otherwise successful with the other log files?  bkp-%lastmonthfiles%.zip still exists, and your script could potentially delete the file that was skipped.

If I may make yet another suggestion, you should rewrite the end of your script to take advantage of 7-zip's exit codes.  Try this instead.  (Note: "%zip%" is in quotes on the assumption that you followed "General practice advice #2" above.)

:compress
"%zip%" -tzip  a -y "bkp-%lastmonthfiles%.zip" %lastmonthfiles%* && (

    DEL /q %lastmonthfiles%*
    echo Zipping complete.  Press any key to exit.
    pause >NUL
    goto :EOF

) || (

    if ERRORLEVEL 2 (

        echo Zipping failed ^(exit status %ERRORLEVEL%^).  Trying again in 5 seconds...

    ) else (

        echo Zip completed with warnings ^(most likely because a file was locked by another
        echo process and had to be skipped^).  Trying again in 5 seconds...

    )

    del "bkp-%lastmonthfiles%.zip" >NUL 2>&1
    ping -n 6 0.0.0.0 >NUL
    goto compress

)

A note about the && and || notation there: That's shorthand code for testing the exit code of the command preceding it.  program.exe && success || fail.  See conditional execution for more details on how this works.


Counting hits by domain I wrote this algorithm which takes input data from something like a CSV file or a large array that contain a series of String elements in the format of "count, FQDN" and then adds or increments the count of each domain component up to the complete FQDN. For example:

// Sample output (in any order/format):
// getTotalsByDomain(counts)
//   1320    com
//   900    google.com
//   410    yahoo.com
//    60    mail.yahoo.com
//    10    mobile.sports.yahoo.com
//    50    sports.yahoo.com
//    10    stackoverflow.com
//     3  org
//     3  wikipedia.org
//     2  en.wikipedia.org
//     1  es.wikipedia.org
//     1  mobile.sports
//   1  sports

let counts = [ "900,google.com",
    "60,mail.yahoo.com",
    "10,mobile.sports.yahoo.com",
    "40,sports.yahoo.com",
    "300,yahoo.com",
    "10,stackoverflow.com",
    "2,en.wikipedia.org",
    "1,es.wikipedia.org",
    "1,mobile.sports" ];

I was able to do this pretty well with the below algorithm, but I am concerned with the inner for loop which uses var 'j'. I felt like the only way that I could incrementally parse the domain components from the already split array was to create another array which unshifted the domain components to partially create a new array until I completed all of the components of the given FQDN element.

function getDomainHits(arr){

  var splitCount = [];
  var domainCountDict = {};

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

    splitCount = arr[i].split(",");

    var curCnt = 0;
          if (splitCount[0]){

            curCnt = splitCount[0];

          }

    var domain = [];
    var currentDom = [];
          if (splitCount[1] != undefined && splitCount[1]){

            domain = splitCount[1].split(".");

            for (var j = domain.length - 1; j >= 0; j--){

               currentDom.unshift(domain.pop());

               /*console.log("current iter: " + k + "\n"
                           + "currentDom: " + currentDom.join(".") + "\n"
                           + "current count: " + curCnt + "\n");*/

              if (currentDom.join(".") in domainCountDict){
                /*console.log("currentDom2: " + currentDom.join("."));
                console.log("increment existing");*/
                domainCountDict[currentDom.join(".")] += parseInt(curCnt);

              }
              if (!(currentDom.join(".") in domainCountDict)){
                /*console.log("currentDom3: " + currentDom.join("."));
                console.log("increment new");*/
                domainCountDict[currentDom.join(".")] = parseInt(curCnt);

                //console.log(domainCountDict);
              }

            }

          }

  }

  return domainCountDict;

}

console.log(getDomainHits(counts));

If you want to see a complete walkthrough of my logic then you can see my answer to my question in Stack Overflow

You can modify the input until there's nothing left of it.  In this example, I've used a regex with .replace() to shorten the FQDN.

If you don't like regex, you could instead split the domain name on ., then in the loop use dom.join(".") to compose the object key, followed by dom.shift() to shorten the array.  With the array technique, the loop conditional is dom.length (instead of just dom).

function getDomainHits(arr){
    let total={};
    arr.forEach( row => {
        let [hits, dom] = row.split(",");
        hits=parseInt(hits);
        while ( hits && dom ) {
            total[dom] = (total[dom] || 0) + hits;
            dom = dom.replace( /^[^.]*\.?/, '' )
        }
    });
    return total;
}


MySQL query: blog entry has comments or not How expensive is this query?

Lets say for example if I don't have a field called total_comments in my blog table. All I want to know is the blog has comments or not.

SELECT EXISTS (SELECT true FROM comments WHERE blog_id = b.id) as has_comments FROM blogs b

How else could this be achieved?

UPDATE: Ah...I've reread your original query. When I first read it, I thought that you were only retrieving this for a single blog, but now I see that you're retrieving for every blog record.

This might be inefficient, though I'm not sure.  I suppose it depends on how good MySQL is at dealing with that subquery.

Below is probably the most straightforward approach:

SELECT
    blogs.blog_id,
    COUNT(comments.blog_id)
FROM blogs
LEFT OUTER JOIN comments on
    comments.blog_id = blogs.blog_id
GROUP BY
    blogs.blog_id

Of course, this gives you a count, which is more information that was asked for, but this will probably be pretty efficient if you have an index on blog_id.

Another alternative is to union together a result of the  blogs that have comments with another result of the blogs that don't have comments.

SELECT blog_id, true
FROM blogs
WHERE
    EXISTS (
        SELECT true
        FROM comments
        WHERE
            comments.blog_id = blogs.blog_id
    )

UNION ALL

SELECT blog_id, false
FROM blogs
WHERE
    NOT EXISTS (
        SELECT true
        FROM comments
        WHERE
            comments.blog_id = blogs.blog_id
    )


Event-based producer/consumer in C# I want to create a production-ready producer/consumer that could help me avoid thread synchronization hell. Is this thread-safe? The main issue is to be safe with exceptions that can arrive.

public class AsynchSimpleProducer<T> : IDisposable
{
    private readonly Action<Exception> error;
    private readonly BlockingCollection<T> blockingCollection = new BlockingCollection<T>(50);

    public event Func<T, Task> NewItem;

    public AsynchSimpleProducer(Action<Exception> error)
    {
        this.error = error;

        Task.Factory.StartNew(() =>
                              {
                                  //this loop is ends only when blockingCollection.CompleteAdding called
                                  Parallel.ForEach(blockingCollection.GetConsumingPartitioner(), SendItem);
                              });
    }

    private void SendItem(T item)
    {
        try
        {
            if (NewItem !=null)
                NewItem(item).Wait();
        }
        catch (Exception ex)
        {
            error(ex);
        }
    }

    public void Send(T newValue)
    {
        blockingCollection.Add(newValue);
    }

    public void Dispose()
    {
        blockingCollection.CompleteAdding();
    }
}

Example of usage:

simpleProducer = new AsynchSimpleProducer<int>(Error);
simpleProducer.NewItem += simpleProducer_NewItem;
//this part can be in task
var next = random.Next(0, 1000);
simpleProducer.Send(next);

I don't really understand how you want to use this so I'm just going to add a couple of comments for you to ponder.

Naming

It's more common to abbreviate Asynchronous to Async not Asynch when used in identifers: AsynchSimpleProducer<T> -> AsyncSimpleProducer<T>.

NewItem isn't a great name for an event, events generally need a verb to make sense e.g. Closed, Clicked, Closing are all good names for an event. Consider something like ItemAdded or similar.

Thread safety

You aren't raising your event safely:

if (NewItem != null)
    NewItem(item).Wait();

NewItem can change between the check and using it. If a listener unsubscribes in this time you'll get an Exception! The proper way to do this is:

var newItem = NewItem;
if (newItem != null)
{
    newItem(item).Wait();
}

Note that I have also added in braces as I personally think they should always be included, even for single line blocks.

Asynchronicity

I had to type something that made me very sad.

newItem(item).Wait();

It's not async if you immediately wait on it. It would be interesting to know what sort of work you're passing in here as it's not obvious whether you need to try to make this asynchronous or not. I'm concerned that you have confused threading and asynchronicity.

Edit

I can't believe I didn't mention this before, I must have been distracted...

If your event handlers throw (the things listening on the NewItem event), you're toast. Say you have 2 listeners, if the first one throws, the second wont be called.

You need to go through each one in the invocation list:

 // untested...
 var runningTasks = new List<Task>();
 foreach (var delegate in newItem.GetInvocationList())
 {
     runningTasks.Add(delegate(item));
 }
 // catch exceptions here.
 Task.WaitAll(runningTasks.ToArray());

I wish I could suggest something useful but I still haven't figure out what you intend on doing with this thing.


Exporting information on a collection of books to Excel I have the following code that loops through a collection of objects and dumps different properties out to Excel.  The whole j++ on every other line doesn't seem very elegant.  Is there a more elegant way to have this functionality where I loop through objects in a collection and dump out properties?

int rowIndex = 2;
foreach (BookInfo book in books)
{
    int j = 1;
    excelExport.SetCell(j, rowIndex, book.BookId);
    j++;
    excelExport.SetCell(j, rowIndex, book.Book);
    j++;
    excelExport.SetCell(j, rowIndex, book.System);
    j++;
    excelExport.SetCell(j, rowIndex, book.Age);
    j++;
    excelExport.SetCell(j, rowIndex, book.StartDate);
    j++;
    excelExport.SetCell(j, rowIndex, book.Pages);
    rowIndex++;
}

I am constantly adding new columns at the beginning and middle, so I want to avoid hard coding column names.

I think your code is perfectly understandable as it is.

Here's an idea though. (In case it is not clear: the suggestions here are more for amusement and edification than a serious suggestion. The original procedural code is just fine, but it is interesting to see how it might be done in a functional style.)

var funcs = new List<Func<BookInfo, object>>()
{
    info=>info.BookId,
    info=>info.Book,
    info=>info.System // etc.
}
int rowIndex = 2;
foreach (BookInfo bookInfo in books)
{
    int columnIndex = 1;
    foreach(var func in funcs)
    {
        excelExport.SetCell(columnIndex, rowIndex, func(bookInfo));
        columnIndex += 1;
    }
    rowIndex += 1;
}

However, this still has a disappointingly large number of variable mutations. Why do we need to have local variables to track the rows and columns at all? That's the inelegant part that you want to eliminate.

  Can't tell if trolling...or just addicted to lambdas

Oh, we're just getting started here. I have barely yet even begun to use lambdas. How about this?

var funcs = new List<Func<BookInfo, object>>()
{
    info=>info.BookId,
    info=>info.Book,
    info=>info.System // etc.
}
var cells = bookInfos.SelectMany(
  (bookInfo, row)=>
    funcs.Select(
      (func, col)=>
        new {row, col, item = func(bookInfo)}));
foreach(var cell in cells)
    excel.SetCell(cell.col, cell.row, cell.item);

There, now we've got a selector that contains a lambda that contains a selector that contains a lambda that iterates over a list of lambdas. We've also gotten rid of every index mutation.

That of course has far too many explanatory local variables. We should be able to do this without mutating any variable except the loop variable.  Let's eliminate all the variable mutations except one:

foreach(var cell in
  bookInfos.SelectMany(
    (bookInfo, row)=>
      new List<Func<BookInfo, object>>()
      {
        info=>info.BookId,
        info=>info.Book,
        info=>info.System // etc.
      }.Select(
        (func, col)=>
          new {row, col, item = func(bookInfo)})))
  excel.SetCell(cell.col, cell.row, cell.item);

And now we've illustrated the old saying: every programming language eventually resembles Lisp -- badly.

As Scott Rippey points out in his answer, we actually don't need to be capturing the properties as lambdas at all; we could just capture the values:

foreach(var cell in
  bookInfos.SelectMany(
    (bookInfo, row)=>
      new object[]
      {
        bookInfo.BookId,
        bookInfo.Book,
        bookInfo.System // etc.
      }.Select(
        (item, col)=>
          new {row, col, item})))
  excel.SetCell(cell.col, cell.row, cell.item);

Which is actually not too bad.

But like I said, your original code is just fine.


Breadth-first search(BFS) Implementation in Python 3 I have implemented a BFS. Does this look like a proper implementation of BFS in Python 3? And if you think something needs to be improved, say it.

class Node(object):

    def __init__(self, name):
        self.name = name
        self.adjacencyList = []
        self.visited = False
        self.predecessor = None

class BreadthFirstSearch(object):

    def bfs(self, startNode):

        queue = []
        queue.append(startNode)
        startNode.visited = True

        # BFS -> queue
        while queue:

            actualNode = queue.pop(0)
            print(actualNode.name)

            for n in actualNode.adjacencyList:
                if not n.visited:
                    n.visited = True
                    queue.append(n)

node1 = Node("A")
node2 = Node("B")
node3 = Node("C")
node4 = Node("D")
node5 = Node("E")

node1.adjacencyList.append(node2)
node1.adjacencyList.append(node3)
node2.adjacencyList.append(node4)
node4.adjacencyList.append(node5)

bfs = BreadthFirstSearch()
bfs.bfs(node1)

There are a few PEP8 issues:

In Python3, class definitions have been overhauled a bit, and one can simply do class Node: instead of class Node(object):.
The spacing is a bit all over the place: there should be two blank lines between class definitions, and I find the spacing within def bfs(self, start_node) difficult.
The naming convention is to be snake_cased.

To address this type of issues, have a look at flake8 or black

Node.visited is problematic.
First, if the node is in several trees, you may not visit it if it's already been  visited in another tree, or even in the same tree when doing a second search.

The fact that you have visited a node is part of the algorithm, and not so much a property of a Node.

With a few tweaks you can rely on queue only, or have a second list of visited:

visited = []
for node in current_node.adjacency_list:
   if not node in visited:
        visited.append(node)

The comment #BFS -> queue is not very telling.

Lastly, this is iterating through the tree, and not doing actual search. Where's the exit condition? What's returned? What should I get when searching for node4 from node1? node4 from node3? node3 from node2?

It's not quite done...


Sudoku solver recursive backtrack in Java I am implementing recursive backtracking algorithm for sudoku. Below is the code to create board with random filling. I wonder if I can make it better in any terms of readable coding especially validation methods. in Board class.

public class Board {
private int[][] board;
public final static int SIZE = 9;

public Board() {
    create();
}

public int[][] getBoard() {
    return board;
}

private void create() {
    int openspots = 20;
    board = new int[SIZE][SIZE];
    Random rand = new Random();
    int random, randomrow, randomcolumn;

    while (openspots-- > 0) {
        random = rand.nextInt(SIZE) + 1;
        randomrow = rand.nextInt(SIZE);
        randomcolumn = (rand.nextInt(SIZE));

        if (board[randomrow][randomcolumn] == 0) {
            board[randomrow][randomcolumn] = random;
            int count = 0;

            while (!isValid(randomrow, randomcolumn)) {
                if (count++ == 9) {
                    board[randomrow][randomcolumn] = 0;
                    openspots++;
                    break;
                }

                random = rand.nextInt(SIZE) + 1;
                board[randomrow][randomcolumn] = random;
            }
        }
    }
}

public void printBoard() {
    for (int row = 0; row < SIZE; row++) {
        for (int column = 0; column < SIZE; column++) {
            System.out.print(board[row][column] + " ");
        }
        System.out.println();
    }
}

public boolean isValid(int row, int column) {
    return isValidRow(row) && isValidColumn(column) && isValidBlock(row, column);
}

private boolean isValidRow(int row) {
    if (row > 8 || row < 0)
        throw new IllegalArgumentException("Row must be between 0 and " + (SIZE - 1) + " inclusive!");

    Set<Integer> set = new HashSet<>();

    for (int i = 0; i < SIZE; i++) {
        if (board[row][i] != 0 && set.contains(board[row][i])) {
            return false;
        } else {
            set.add(board[row][i]);
        }
    }

    return true;
}

private boolean isValidColumn(int column) {
    if (column > 8 || column < 0)
        throw new IllegalArgumentException("Column must be between 0 and " + (SIZE - 1) + " inclusive!");

    Set<Integer> set = new HashSet<>();

    for (int i = 0; i < SIZE; i++) {
        if (board[i][column] != 0 && set.contains(board[i][column])) {
            return false;
        } else {
            set.add(board[i][column]);
        }
    }

    return true;
}

private boolean isValidBlock(int row, int column) {
    if (column > 8 || column < 0)
        throw new IllegalArgumentException("Column must be between 0 and " + (SIZE - 1) + " inclusive!");

    int blockrow = (row / 3) * 3;
    int blockcolumn = (column / 3) * 3;

    Set<Integer> set = new HashSet<>();

    for (int i = 0; i < 3; i++) {
        for (int j = 0; j < 3; j++) {
            if (board[blockrow + i][blockcolumn + j] != 0 && set.contains(board[blockrow + i][blockcolumn + j])) {
                return false;
            } else {
                set.add(board[blockrow + i][blockcolumn + j]);
            }
        }
    }

    return true;
}
}

The one below is the solver class which has the main algorithm to solve sudoku.

public class Solver {

public boolean solve(Board board) {
    int[][] boardarr = board.getBoard();

    for (int i = 0; i < board.SIZE; i++) {
        for (int j = 0; j < board.SIZE; j++) {
            if (boardarr[i][j] == 0) {
                for (int k = 1; k <= board.SIZE; k++) {
                    boardarr[i][j] = k;
                    if (board.isValid(i,j) && solve(board)) {
                        return true;
                    }
                    boardarr[i][j] = 0;
                }
                return false;
            }
        }
    }

    return true;
}
}

Object Orientated Programming

you don't use object orientated programming! that would clean up the code in a large scale

public class Board {
    //private int[][] board; primitive obsession - use objects instead
    private Field[][] fields;
    private Column[] columns;
    private Row[] rows;
    private Block[] blocks;
    public final static int SIZE = 9;

    ...
}

you could provide the proper methods for the field - so you could easily represent the field - instead of trying to interpret an int value.

you could provide methods that are specific for these objects - you could even define an interface for common code, like isFieldValid(Field candidate) for Row, Column and Block

Naming

consider these names and you might find better

Random rand = new Random();
int random, randomrow, randomcolumn;

maybe it would be

Field candidate;

again here, in Java we don't use the hungarian notation and name a variable by it's type:

//int[][] boardarr = board.getBoard();
int[][] board = board.getBoard(); //would be better
Field[][] fields = board.getFields(); //even more better

Data Structure

if you would use Objects for your Sudoku-Solver you could re-use values (here an example snippet for Block)

class Block {
    final private Set<Field> fields = new HashSet<>();

    boolean isFieldValid(Field field) {
        //Set<Integer> set = new HashSet<>(); we already have such set
        //and don't need to create a new one whenever we check
        return !fields.contains(field);
    }
}

Magic numbers

you already provide a SIZE = 9 constant - so why don't you use a proper one for the block size?

int blockrow = (row / 3) * 3;
int blockcolumn = (column / 3) * 3;

Testing

i assume you provide enough tests but just don't put these in the question

Summary

very nice piece of code - i'm glad you have posted it here! The algorithm seems straight forward, i like it! The OO-thing is the only flaw here. I would appreciate if you would provide another question with applied OOP!


Adding bits, simpler Considering that two bitsets are provided in the form of strings, char a[] and char b[], as in the code below. And the task of add that bitsets is given. I take it and wrote the following to sum the two bitsets:

#include <stdio.h>
#include <string.h>

int carry = 0;
char result(char r, int c) {
    char R = '0';
    if (carry) {
        if (c) {
            R = '0';
            carry = 1;
        } else {
            R = r == '0' ? '1' : '0';
            carry = R == '0' ? 1 : 0;
        }
    } else {
        if (c) {
            R = '0';
            carry = 1;
        } else {
            R = r;
            carry = 0;
        }
    }
    return R;
}

char sum(int x, int y) {
    char r = '0';
    if (x ^ y)
        r = result('1', 0);
    else if (x & y)
        r = result('0', 1);
    else
        r = result('0', 0);
    return r;
}

int main() {
    char a[] = "01";
    char b[] = "01";
    char c[] = "00";
    for (int i = strlen(c)-1; i > -1; i--)
        c[i] = sum(a[i] == '0' ? 0 : 1, b[i] == '0' ? 0 : 1);
    printf("%s\n", c);
}

Is it possible to make it simpler?

OP's approach is very tangled.

No provisions are made for a final carry-out of the sum.

char result(char r, int c) { should at least been used description names: char add_digit_carry(char digit, int carry) {

for (int i = strlen(c)-1; i > -1; i--) --> Little things like i >= 0; are certainly easier to follow.

strlen() returns type size_t.  Array indexing and sizing is best done with that type.

The basic way to add strings is just like grade-school math:  add the 1 places, form the sum and carry.  Then add the next most significant column along with the carry.  Repeat for all digits and consider a carry out.

Assuming memory issues are not a big concern ....

#include <assert.h>
#include <stdlib.h>
#include <string.h>

// c = a + b
char *KFM_add(char *c, size_t c_size, const char *a, const char *b, int base) {
  assert(base >= 2 && base <= 10 && c_size > 0);
  size_t a_end = strlen(a);
  size_t b_end = strlen(b);
  size_t c_end = c_size - 1;

  c[c_end] = '\0';
  int carry = 0;

  while (a_end > 0 || b_end > 0 || carry > 0) {
    int sum = carry;
    if (a_end > 0) {
      sum += a[--a_end] - '0';
    }
    if (b_end > 0) {
      sum += b[--b_end] - '0';
    }
    assert(c_end > 0);
    c[--c_end] = sum % base + '0';
    carry = sum / base;
  }

  // return memmove(&c[0], &c[c_end], c_end); // Corrected code @JS1
  return memmove(&c[0], &c[c_end], c_size - c_end);
}

#include <stdio.h>

int main(void) {
  char a[] = "111";
  char b[] = "01";
  char c[80];

  puts(KFM_add(c, sizeof c, a, b, 2));  // Output 1000
}

Should one want to explore binary addition closer:

#include <stdbool.h>
  ...
  bool carry = 0;
  while (a_end > 0 || b_end > 0 || carry > 0) {
    bool a_bit = (a_end > 0) ? (a[--a_end] - '0') : false;
    bool b_bit = (b_end > 0) ? (b[--b_end] - '0') : false;
    assert(c_end > 0);
    c[--c_end] = (a_bit ^ b_bit ^ carry) + '0';
    carry = (a_bit & b_bit) | (b_bit & carry) | (carry & a_bit);
  }
  ...


Two maximal value in array. Sorted and Array.Sort() Compare the two methods of sorting the array in ascending order, where can I optimize? How best to handle exception handling?

    class Inside {

    Random rand = new Random();

        // step 1.1: Fill array, output array

        public void SortedArray(int arrSize) {

            int[] arrOne = new int[arrSize];
            Console.Write("Array size: " + arrSize + "\n\t\t\t\t\t\tARRAY\n");

            for (int x = 0; x < arrOne.Length; x++) {

                arrOne[x] = rand.Next(-1000, 1000);
                Console.Write("{0, 8} | ", arrOne[x]);
                if (x % 8 == 0) Console.WriteLine();
            }

            // step 1.2: Sorted array and output on monitor two maximal value

            for (int i = 0; i < arrOne.Length; i++) {

                int minIndex = i;
                for (int j = i + 1; j < arrOne.Length; j++) if (arrOne[minIndex] > arrOne[j]) minIndex = j;
                int temp = arrOne[minIndex];
                arrOne[minIndex] = arrOne[i];
                arrOne[i] = temp;
            }

            Console.Write("\n\n\t\t\t\t\t\tARRAY SORTED\n\n");

            for (int x = 0; x < arrOne.Length; x++) {
                Console.Write("{0, 8} | ", arrOne[x]);
                if (x % 8 == 0) Console.WriteLine();
            }

            Console.WriteLine("\n\nMaximal value 1: " + arrOne[arrOne.Length - 1] + "\nMaximal value 2: " + arrOne[arrOne.Length - 2]);
        }

        // Step 2.1: Fill array and output

        public void SortedMethod(int arrSize) {

        int[] arrOne = new int[arrSize];
        Console.Write("Array size: " + arrSize + "\n\t\t\t\t\t\tARRAY\n");

        for (int x = 0; x < arrOne.Length; x++)  {

            arrOne[x] = rand.Next(-1000, 1000);
            Console.Write("{0, 8} | ", arrOne[x]);
            if (x % 8 == 0) Console.WriteLine();
        }

        // Step 2.2: Sorted array and output on monitor two maximal value

        Array.Sort(arrOne);
        Console.Write("\n\n\t\t\t\t\t\tARRAY SORTED\n\n");

        for (int x = 0; x < arrOne.Length; x++)  {
            Console.Write("{0, 8} | ", arrOne[x]);
            if (x % 8 == 0) Console.WriteLine();
        }
        Console.WriteLine("\n\nMaximal value 1: " + arrOne[arrOne.Length - 1] + "\nMaximal value 2: " + arrOne[arrOne.Length - 2]);
    }
}

// user class

class Data {

static void Main() {

        // Step 1: Call the sort method by selecting
        Inside obj = new Inside();
        Console.Write("Please, enter array size: ");
        int sizeArr = Convert.ToInt32(Console.ReadLine());
        var timeStart = Stopwatch.StartNew();
         obj.SortedArray(sizeArr);
         timeStart.Stop();

        // Step 2: Call method Array.sort()

        var timeStartTwo = Stopwatch.StartNew();
        obj.SortedMethod(sizeArr);
        timeStartTwo.Stop();

        // Step 3: Compare runtime

        Console.Write("SortedArray = {0}\nSortedMethod = {1}", timeStart.Elapsed, timeStartTwo.Elapsed + "\n");
    }
}

for (int j = i + 1; j < arrOne.Length; j++) if (arrOne[minIndex] > arrOne[j]) minIndex = j;

This is by far the worst line in your code. It took me a while to realize that there is actually an if behind the for while I was wondering why didn't you indent the three lines below it but wait, these lines are not part of the for, it's the if. You should always use curly brackets

for (int j = i + 1; j < arrOne.Length; j++)
{
    if (arrOne[minIndex] > arrOne[j])
    {
        minIndex = j;
    }
}

This is how it should look like.


Parsing command input with simple bimaps I've been playing with a simple toyrobot exercise to brush up some of my forgotten language skills and was wondering if there was a nicer way to implement my main command parsing loop.  I think the use of the two maps and the nested ternary operator is readable but would be interested in seeing other approaches.

#include "toyrobot.hpp"
#include <iostream>
#include <regex>
#include <string>
#include <map>

int main () {
  using namespace std;

  map<string,int> dirtoi = {{"NORTH", 0}, {"EAST", 1}, {"SOUTH", 2}, {"WEST", 3}};
  map<int,string> itodir = {{0, "NORTH"}, {1, "EAST"}, {2, "SOUTH"}, {3, "WEST"}};
  regex place("^PLACE (\\d),(\\d),(NORTH|EAST|SOUTH|WEST)$");

  Robot robot = Robot();
  Option<Table> table = Some(Table(Point(0,0),Point(4,4)));

  smatch m;
  string l;
  while (getline(cin, l))
  {
        l == "MOVE" ? robot.move() :
        l == "LEFT" ? robot.left() :
        l == "RIGHT" ? robot.right() :
        l == "REPORT" ? robot.report(itodir) :
        regex_match (l,m,place) ? robot.place(Point(stoi(m[1]), stoi(m[2]))
                                            , dirtoi.at(m[3])/2.0, table) :
    (void)0;
  }
}

Here are some things I saw and some suggestions for how you might improve your code.

Try not to repeat information

I don't much care for the two parallel maps because they have the same information but in two different places and in two different constructs.  What you actually seem to need is a way to go from a std::string to an int and vice-versa.  For that, I'd write a simple class that does this:

template <class T>
class Bimap
{
public:
    Bimap(std::initializer_list<T> init) : names{init} {}
    const T& operator[](int i) const { return names[i]; }
    int operator[](const T& n) const {
        for (unsigned i=0; i < names.size(); ++i) {
            if (names[i] == n)
                return i;
        }
        return -1;
    }
private:
    std::vector<T> names;
};

Note that is, in fact, a templated class so it could be used with std::string or std::wstring or any other class that implements operator== and can be placed in a std::vector.  The simple linear search is not as efficient as a std::map but it's probably just fine for short lists.  Using it in main is just this line:

const Bimap<string> dir{{"NORTH", "EAST", "SOUTH", "WEST"}};

Use const where practical

Your regex and std::map variables could and should be const.

Separate parsing from actions

The code is actually attempting to do two different things:  it is parsing a line from input and then calling some action based on what it finds.  It's probably useful to separate those two a little more clearly.  I'd probably use flex and bison if the commands become more complex, but for now you can simply use a more complete regex.  Here's one way to do that:

const Bimap<string> simplecmd{{"MOVE", "LEFT", "RIGHT", "REPORT"}};
enum scmd { MOVE, LEFT, RIGHT, REPORT };
const regex command("^(MOVE|LEFT|RIGHT|REPORT)|PLACE (\\d),(\\d),(NORTH|EAST|SOUTH|WEST)$");

By reusing the Bimap class defined above, we can change any command into an integer.  By expressing all possible commands within a regex, we can simply ignore any line which doesn't match (or perhaps print an error message).

I'm not happy about having the parallel constructs of simplecmd and the enum scmd but couldn't think of an easy way to avoid it in this case.  We can rationalize it by saying that if one wished to have, say, both French and English versions, only the simplecmd would need to changed and not the enum.

Use better variable names

Generally, single letter variable names other than i and j for loop variables, are best avoided in favor of longer names.  The variable m is not too terrible, but the variable l is definitely a poor choice, not least because it's easily mistaken for the digit 1 or the letter i.  I changed it to line in the rewrite I did.

Use a switch statement where appropriate

Because your parser is selecting some action based on the input, we can make that much more explicit (and easier to read and maintain) by using a switch statement instead of the chained ternary operator.  Here's one way to do it, incorporating all of the other suggestions:

smatch m;
string line;
while (getline(cin, line))
{
    if (regex_match(line, m, command)) {
        switch(simplecmd[m[1]]) {
            case MOVE:
                robot.move();
                break;
            case LEFT:
                robot.left();
                break;
            case RIGHT:
                robot.right();
                break;
            case REPORT:
                robot.report();
                break;
            default:   // must be PLACE
                robot.place(Point(std::stoi(m[2]), std::stoi(m[3])), dir[m[4]], table);
        }
    }
}