Mapping object values from one object to another I have an object called defaultOptions and a function that accepts as an argument an object called newValues meant to override the values in defaultOptions with its own.  The values that are not specified in newValues should remain default.

var configOptions = {
    isActive: false,
    name: "",
    description: "",
    category: "",
    group: "default"
}

    publishOptions: function(newValues){
      $.each(newValues, function(key, value){
        if(configOptions.hasOwnProperty(key)){
          configOptions[key] = value;
        }
        else{
          configOptions[key] = value;
        }
      });
      console.log(configOptions);
    }

For example, if I use this method like so:

var myOptions = {
    name: "option one",
    category: "the best"
}

publishOptions(myOptions)

I would expect configOptions to look like this:

configOptions = {
    isActive: false,
    name: "option one",
    description: "",
    category: "the best",
    group: "default"
}

I'm using jQuery's each method to map the values from newValues to configOptions, but I suspect there is a much more succinct and elegant way to do this.  I would appreciate any suggestions.

jQuery has extend, which seems to be what you are looking for:

http://api.jquery.com/jquery.extend/

Otherwise, if you don't want to use jQuery at all, you can make a for loop:

var first  = { value: 1, string: "two" };
var second = { value: 2 };

for ( var i in second )
  if ( first.hasOwnProperty( i ) )
    first[i] = second[i];

first.value == 2; // true

As a side, your if statement isn't doing anything because both cases assign the value to configOptions.


Validating an arithmetic sequence I'm looking for criticism on the following method used to determine if a collection of Integers forms an arithmetic sequence:

    public boolean isArithmetic(Integer[] nums)
{
    Set<Integer> result = new HashSet<Integer>();
    Collections.sort(Arrays.asList(nums));
    for(int i = 0; i < nums.length -1; i++)
    {
        result.add(nums[i+1] - nums[i]);
    }

    if(result.size() > 1)
        return false;
    return true;
}

The basic method you're using seems to me rather roundabout. If you're going to write a loop to walk through the collection anyway, why not just check for the correct criteria there? e.g., something like this:

Integer diff = nums[1] - nums[0];

for (int i=2; i<nums.length; i++)
    if (nums[i] - nums[i-1] != diff)
        return false;
return true;

This can save quite a bit of work, and (especially if the numbers don't form an arithmetic sequence) may save a fair amount of storage as well. More importantly, it makes the intent immediately obvious, which (it seems to me) that inserting all the differences into a hash table doesn't.

One other specific detail of your code bothers me:

if(result.size() > 1)
    return false;
return true;

Anything of the form if (x) return true; else return false; can be expressed as return x;. In your case, the sense is reversed, so it can be return !x;, so you just want return result.size() == 1; (but, as noted above, I'd rather eliminate this entirely).

If you really want to use the approach you've taken, I think it becomes a lot more reasonable if you take your loop and wrap it up into a generic algorithm. I'd probably call it adjacentDifference. Then you end up with something on this general order:

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

Collections.sort(Arrays.asList(nums));
Algorithms.adjacentDifference(nums, result);
return result.size() == 1;

That's simple and neat enough that as long as you're sure this is outside the critical path (i.e., the extra time and space used don't matter) it would at least be worth considering.


Is building a test instance from a mix of both mock and real objects OK? I am new to android testing and would like to try start off in the correct direction, so I am trying to understand if this is the correct way to test a particular method or if there is some best practice that I should be following.

Here are some snippets of the class that contains the method I would like to test.

public class MapMarker implements Target {

private Bus mBus;
private RailsMarker mMarker;
private Bitmap mBitmap;

public String getGravatarUrl() {
    return mMarker.getGravatarUrl();
}

public String getUserId() {
    return mMarker.getUserId();
}

@Override
public void onBitmapLoaded(Bitmap bitmap, LoadedFrom loadedFrom) {
    System.out.println(String.format("loaded bit map from gravatar url = %s, for userid = %s",getGravatarUrl(), getUserId()));
    mBitmap = bitmap;
    mBus.post(new MarkerReadyEvent(this));
    }
}

I am trying to test that when the onBitMapLoaded method is called, it posts a MarkerReadyEvent onto my eventbus.

Here is the test method I coded.

@Test
public void testOnBitMapLoaded() {
    Bus mockBus = mock(Bus.class);
    RailsMarker railsMarker = new RailsMarkerBuilder().withGravatarUrl("a_gravatar_url")
                                                      .withUserId("a_user_id")
                                                      .build();
    MapMarker androidMapMarker = new MapMarkerBuilder().withBus(mockBus)
                                                       .withMarker(railsMarker)
                                                       .build();
    androidMapMarker.onBitmapLoaded(null, null);
    verify(mockBus).post(isA(MarkerReadyEvent.class));
}

I am using Mockito to mock the eventbus.
I am creating a real instance of the MapMarker class (via https://github.com/mkarneim/pojobuilder)
I am also creating a RailsMarker instance to be included in the MapMarker instance.  (I realized this in only needed for the println statement but I did not want to remove it just to make the test easier)

Is this a good approach or is there some other pattern I should be following?
The part that seems a bit strange is that I end up building the instance I am going to test with both a mock object (the Bus) and a real object (the RailsMarker),  but I don't see anyway around this.

You would be testing the MapMarker more in isolation if you would inject only mocks.

Isolation is important for two things:

stability of test results
more explicit and direct feedback

If a bug would be introduced in the RailsMarker, this test would also fail, causing it to be less stable.
In case of this bug, there would by multiple test failures (also for RailsMarker and possibly other tests that use this class) making it harder to find the bug.

In general I would use mocks for any dependant object that has more logic than just getters and setters. That being said, it looks like the RailsMarker might qualify.


Writing huge 2D vectors to text file too slow I have 10 large (34 million cell) 2D gridded vectors storing doubles. When written they are over 200 MB in size. I use an ofstream object to write them to a text file (.csv format), one element at a time, using two for loops (one for my rows, one for my columns). They take way to long to write. Is there a faster way to write from these vectors?

(I am using Visual Studio 2012.)

// Resizing of vectors
flowDirGrid.resize(rows, std::vector<double>(cols, NODATA));

// Do some processing
....

// Outputting processed data
ofstream outfile3("FlowDir.dat");
if(!outfile3.good())
    return;

for(int i=0; i<rows; i++)
    {
        for (int j=0; j<cols; j++)
        {
            if(elevation[i][j]!=NODATA)
                outfile3 << flowDirGrid[i][j]<<" ";
            else
                outfile3 << NODATA<<" ";
        }
        outfile3 << std::endl;
    }

outfile3.close();

The output-writing code should reside in a function that takes two parameters: the flowDirGrid and a filename.

Don't omit braces on multi-line conditionals.  It's a filthy habit, and someday you will contribute to a coding accident like this.

A performance problem is outputting std::endl at the end of every line, which causes a flush.  Output '\n' instead.


Compress an array of strings in Android I'm compressing an array of strings in an Android app. Is there something you would suggest?

public static String compressArray(String[] data){
    String jsonData = Arrays.toString(data);

    try {
        String compressedData = compressString(jsonData);
        int compressedLength = compressedData.length();
        int normalLength = jsonData.length();
        return normalLength < compressedLength? jsonData:compressedData;
    } catch (IOException e) {
        e.printStackTrace();
        return "";
    }

}
public static String compressString(String srcTxt) throws IOException {
    ByteArrayOutputStream rstBao = new ByteArrayOutputStream();
    GZIPOutputStream zos = null;
    try {
        zos = new GZIPOutputStream(rstBao);
        zos.write(srcTxt.getBytes());
        zos.finish();

        byte[] bytes = rstBao.toByteArray();
        return Base64.encodeBase64String(bytes);

    }
    finally {
        zos.close();
        rstBao.close();

    }

}

Compressed vs. non

        return normalLength < compressedLength? jsonData:compressedData;

Isn't this backwards?  Consider

        return normalLength <= compressedLength? jsonData : compressedData;

or

        return compressedLength < normalLength ? compressedData : jsonData;

Both alternatives do the same thing.  The second version is one character shorter and may be a little clearer about what it is doing.  If the compressedLength is less than the normalLength substitute the compressedData for the normal data.

The original returns a compressed version, which has to be uncompressed, when the compressed and the uncompressed data are the same length.  This version returns the uncompressed version in that case.  Same space, but less time (because it can just use the data rather than decompressing it first).

It's not going to be a big deal either way.  Not only is this likely to be rare, it has little impact when it does happen.  It just struck me as being off and easy to fix.

What does close actually do?

    GZIPOutputStream zos = null;
    try {
        zos = new GZIPOutputStream(rstBao);
        zos.write(srcTxt.getBytes());
        zos.finish();

        byte[] bytes = rstBao.toByteArray();
        return Base64.encodeBase64String(bytes);

    }
    finally {
        zos.close();
        rstBao.close();

    }

You don't need to close rstBao for two reasons.

First, look at the Javadoc for close:

  Writes remaining compressed data to the output stream and closes the underlying stream.

Note the last part.  The "underlying stream" in this case is rstBao.  So closing zos automatically closes rstBao for you.

Second, if you look at the Javadoc for ByteArrayOutputStream, the close method:

  Closing a ByteArrayOutputStream has no effect. The methods in this class can be called after the stream has been closed without generating an IOException.

So it is entirely unnecessary to close rstBao.  But you are effectively doing it twice:  once implicitly by closing zos and once explicitly.

The smallest change would drop the second close, but a larger change may better use the notions of the try format.  Consider

    try (GZIPOutputStream zos = new GZIPOutputStream(rstBao)) {
        zos.write(srcTxt.getBytes());
    }

    byte[] bytes = rstBao.toByteArray();
    return Base64.encodeBase64String(bytes);

This moves the byte array manipulations out of the try block, as they don't throw exceptions nor do they require the compression stream.

Moving them out also means that it's not necessary to call finish explicitly.  When the try-with-resources finishes, it will automatically close the the stream and write the data.

This still closes rstBao implicitly, but as discussed it won't do anything.  This is how a ByteArrayOutputStream is designed to operate.  It ignores the close the format requires and continues.


The most efficient way to merge two lists in Java I am looking for a way to merge two files with a list of distinct words, one word per line. I have to create a new txt file that would contain all the words of the first list and all the words from the second list. I don't have any other specifications.  The order of words in the result doesn't matter.

public class testMain {

    public static void main(String[] args) {
        File f1=new File("words.txt");
        File f2=new File("words1.txt");
        HashSet <String> hash1=new HashSet<String>();
        HashSet <String> hash2=new HashSet<String>();
        try{
            Scanner s=new Scanner(f1);
            while(s.hasNextLine()){
                hash1.add(s.nextLine());
            }
            s=new Scanner(f2);
            while(s.hasNextLine()){
                hash2.add(s.nextLine());
            }
        }
        catch(FileNotFoundException e){}
        hash1.addAll(hash2);
        Object[]array =hash1.toArray();
        File newFile=new File("mixOfLists.txt");
        try{
        PrintWriter writer=new PrintWriter(newFile);
        for(int i=0; i<array.length; i++){
            writer.println(array[i]);
        }
            writer.close();
        }

        catch(FileNotFoundException e){ System.out.print("No Such File");}
        System.out.print("Done!");
        }

}

Exception Handling

Your handling is not great.... this is a sign of poor forward planning:

    catch(FileNotFoundException e){}

And this is a sign of something almost as bad:

    catch(FileNotFoundException e){ System.out.print("No Such File");}
    System.out.print("Done!");
    }

The first time I read that, I got confused and thought the "Done!" println was part of the exception handling. You need to work on the indentation. Also, just printing "No such file" is not a very helpful exception handling.

Style

1-liner blocks are seemingly convenient  but in the long term can have negative impacts on maintainability. You have a lot of them, and they make reading your code hard.

Your code is also suffocating due to lack of whitespace. You need to put spaces around operators to help the code to breath.... yeah, that sounds alarmist, but it really helps.

    File newFile=new File("mixOfLists.txt");
    try{
    PrintWriter writer=new PrintWriter(newFile);
    for(int i=0; i<array.length; i++){
        writer.println(array[i]);
    }

should be:

    File newFile = new File("mixOfLists.txt");
    try{
        PrintWriter writer = new PrintWriter(newFile);
        for(int i = 0; i < array.length; i++) {
            writer.println(array[i]);
        }
        ....

Resources

You should use try-with-resources for your IO sources and sinks. As things stand at the moment, you don't close the readers properly.

Algorithm

You're reading both files in to their own sets, and then merging the sets, and then outputting the result.

A better solution would be to use the boolean return value from the add(...) method to determine whether the word has been seen before... consider:

        while(s.hasNextLine()){
            String line = s.nextLine();
            if(hash1.add(line)) {
                writer.println(line);
            }
        }

The above code can be used for both the input files, and only writes out the word if the word has not been seen before.

This way you have only one set, and you do the merge at the same time as the reading.

Also, you should be using Java 8 streams..... hmmm... that would be nice.


Iconography - How would you improve this? I would like your opinion on the following subject.
I have to provide icons in different sizes and states. The sizes will be 16*16, 24*24 and 32*32.

The states bright, dark, disabled and link.

Because of browser support we decided for .png instead of .svg.
Do you feel this is the best CSS approach for this case?

Would you make some changes to improve this styles?

<i class="fx-icon fx-icon-home">

.fx-icon { /*standard icon size*/
    border: none;
    display: inline-block;
    height: 16px;
    line-height: 16px;
    margin-right: 3px;
    vertical-align: top;
    width: 16px;
}

.fx-icon.fx-icon-medium {
    height: 24px;
    line-height: 24px;
    width: 24px;
}

.fx-icon.fx-icon-large {
    height: 32px;
    line-height: 32px;
    width: 32px;
}

.fx-icon.fx-icon-larger {
    height: 48px;
    line-height: 48px;
    width: 48px;
}

/*Icon Home*/
.fx-icon-home-bright,
.fx-icon-home-dark,
.fx-icon-home-disabled,
.fx-icon-home-link {
    background: url(images/icon-home.png) no-repeat;
}

.fx-icon-home-dark {
    background-position: -16px 0;
}

.fx-icon-home-disabled {
    background-position: -32px 0;
}

.fx-icon-home-link {
    background-position: -48px 0;
}

My recommendation would be to use a tool to generate a sprite sheet + CSS, rather than writing it out manually yourself.  I've not used it myself, but the sprite helpers found in Compass (see: http://compass-style.org/reference/compass/helpers/sprites/, but it only works with PNGs) are very popular.  If your set of icons expands in the future, you'll be thankful that you don't have to go back and fix everything manually:  just run your tool again with the new images and you're good to go.

Just because browser support isn't perfect, that doesn't mean you can't have both PNG and SVG.  The only browsers that are widely used that do not support SVGs as a background image are IE8 and Android 2.3x.

By taking advantage of the fact that the browsers that do not support SVGs as a background image either do not support the background-size property (IE8) or require a prefix for it (Android 2.3x), you can serve the correct file to the correct browser:

.foo {
    background: url(foo.png) no-repeat; /* IE8 and Android 2.3x */
    background: url(foo.svg) 0 0 / auto auto no-repeat; /* everyone else */
}

In addition to being able to resize gracefully, SVGs have one other little-known advantage:  you can use media queries inside them.  This is an excellent way of reducing details as the SVG gets smaller or adding details as the SVG gets bigger.  When used as a background image, support for media queries inside the SVG is somewhat limited (the demo below appears to only work as expected in IE9+ and Chrome)

http://codepen.io/cimmanon/pen/HldKw

Foo

Foo

Foo

p {
  display: inline-block;
}

p:before {
  background: url(http://people.opera.com/andreasb/demos/demos_svgopen2009/update/circle.svg) 0 0 / cover no-repeat;
  height: 200px;
  width: 200px;
  display: block;
  content: ' ';
}

.two:before {
  height: 100px;
  width: 100px;
}

.three:before {
  height: 50px;
  width: 50px;
}

The SVG:

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="400" height="400">
 <g>
  <title>Simple SVG + mediaqueries</title>
  <defs>

  </defs>
  <circle cx="200" cy="200" r="150" id="circle" />
 </g>
</svg>

Learn more:  http://blog.cloudfour.com/media-queries-in-svg-images/


Mutable named tuple with default value and conditional rounding support I have following code to create a mutable namedtuple. based my understand I can use dataclass to do it. is there a better way to do it or clean up the code?

@dataclass
class Price:
    """
    This describes how to map default price value for product
    """
    profit: float = 0.5
    cost: float = 0.1
    sale: float = 25.0

    def round(self, n: float):
        if n < 1:
            return round(n, 2)
        elif n < 100:
            n = round(n / 1)
        elif n < 1000:
            n = round(n / 5) * 5
        elif n < 10000:
            n = round(n / 50) * 50
        else:
            n = round(n / 500) * 500
        return n

    def update(self, **kwargs):
        rate = kwargs.get('rate', 1)
        for k, v in asdict(self).items():
            if k != 'sale':
                v = self.round(v * rate)
            v = kwargs.get(k) or v
            setattr(self, k, float(v))

#run test
p = Price()
p.update(rate=1) #p = Price(profit=0.5, cost=0.1, sale=25.0)

p = Price()
p.update(**{sale=450}, **dict(rate=0.9073)) #p = Price(profit=0.45, cost=0.1, sale=450.0)

p = Price()
p.update(**{sale=800}, **dict(rate=301.377)) #p = Price(profit=150.0, cost=0.1, sale=800.0)

p = Price()
p.update(rate=301.377) #p = Price(profit=150.0, cost=0.1, sale=7550.0)

p = Price(0.0, 0.5, 50.0)
p.update(rate=1) #p = Price(profit=0.0, cost=0.5, sale=50.0)

p = Price(0.0, 0.5, 50.0)
p.update(**{sale=600}, **dict(rate=0.9073)) #p = Price(profit=0.0, cost=0.5, sale=600.0)

p = Price(0.0, 0.5, 50.0)
p.update(**{sale=1200}, **dict(rate=301.377)) #p = Price(profit=0.0, cost=0.5, sale=1200.0)

p = Price(0.0, 0.5, 50.0)
p.update(rate=301.377) #p = Price(profit=0.0, cost=0.5, sale=15000.0)

In short:

dataclass is the right thing to use as a mutable named tuple. It's basically custom-built to be a great version of that idea.
Use decimal.Decimal for any financial numbers.
Don't re-implement round. There's a built-in version that behaves in nice, configurable ways with the Decimal object.
Not sure what the p.update(**{sale=600}, **dict(rate=0.9073)) is all about, just use p.update(sale=600, rate=0.9073) (except with Decimals)


Batch file for looking through text files I was creating a program and I was trying to explain it to some of my coding friends. They commented that it was too tedious typing out all the "if" commands. How do I streamline this code?
Note: this is a ".bat" program.
Waiter.bat
@echo off
set /p Food="What would you like to eat? "
if /i %Food%==pizza findstr "Pizza" Menu.txt
if /i %Food%==bread findstr "Bread" Menu.txt
if /i %Food%==macaroni findstr "Macaroni" Menu.txt
pause

Using the user input, the batch file looks inside Menu.txt and then it echos that description for that one food item.
Menu.txt
Pizza | Flattened dough, layered with tomato sauce, cheese, and choice of toppings.
Bread | Dough that has been baked.
Macaroni | Macaroni pasta with cheese.

With the help of a Windows user in chat (Thank's Simon!), we've got a much better solution figured out:

@echo off
set /p Food="What would you like to eat? "
set "Food=%Food% |"
findstr /C:"%Food%" Menu.txt
pause

This will concatenate the pipe character to the end of the user-entered string and find the line that begins with that literal string.

Ordinarily, findstr expects a space delimited list of words to search for.  Using the /C: flag will look for the literal string rather than breaking the string up into separate strings and finding each individual word.

As long as your Menu.txt file maintains the format "MenuItem | Description", this .bat file will work and you won't have to edit it each time you edit your menu.

You just have to keep the formatting and don't put the pipe character anywhere else in the file.


Tower of Hanoi in NASM Win64 Assembly This program is an implementation of the classic puzzle Tower of Hanoi! This is my first mostly recursive Assembly program. Any advice and all topical comments on code optimization and conventions is appreciated!

Compiled as follows using VS2017 x64 Native Tools Command Prompt:

> nasm -g -fwin64 hanoi.asm

> cl /Zi hanoi.obj msvcrt.lib legacy_stdio_definitions.lib

hanoi.asm

;; bits 64
default rel

extern printf, scanf

section .text
    global main
    global hanoi
main:
    sub     rsp, 56
    lea     rcx, [prompt]
    call    printf
    lea     rdx, [rsp+32]
    lea     rcx, [scan_fmt]
    call    scanf
    cmp     eax, 1
    jz      .call_hanoi
    lea     rcx, [scan_fail]
    call    printf
    mov     eax, 1                      ; return 1
.end:
    add     rsp, 56
    ret
.call_hanoi:
    mov     ecx, dword [rsp+32]         ; ecx = num_disks
    mov     r9d, 3                      ; r9d = tmp
    mov     r8d, 2                      ; r8d = dst
    mov     edx, 1                      ; edx = src
    call    hanoi                       ; hanoi(num_disks, src, dst, tmp)
    xor     eax, eax                    ; return 0
    jmp     .end
hanoi:
    push    r12
    mov     r12d, r8d                   ; r12d = r8d = dst
    push    rbp
    push    rdi
    push    rsi
    mov     esi, edx                    ; esi = edx = src
    push    rbx
    sub     rsp, 40
    cmp     ecx, 1                      ; if num_disks == 1
    jz      .skip_recursive_move
    mov     ebx, ecx
    mov     edi, r9d                    ; edi = r9d = tmp
.move_disk:
    lea     ebp, [rbx-1H]               ; ebp = num_disks - 1
    mov     r9d, r12d                   ; r9d = r12d = dst
    mov     r8d, edi                    ; r8d = edi = tmp
    mov     edx, esi                    ; edx = esi = src
    mov     ecx, ebp                    ; ecx = edp = num_disks - 1
    call    hanoi                       ; tmp & dst are swapped for this call
    mov     edx, ebx                    ; edx = ebx = num_disks
    mov     r9d, r12d                   ; update r9d after hanoi call
    mov     r8d, esi                    ; update r8d after hanoi call
    lea     rcx, [prompt_move_disk]
    mov     ebx, ebp                    ; ebx = ebp = num_disks - 1
    call    printf
    cmp     ebp, 1                      ; if num_disks == 1
    jz      .move_one_disk
    mov     eax, esi
    mov     esi, edi
    mov     edi, eax                    ; swaps tmp and src (esi and edi)
    jmp     .move_disk
.skip_recursive_move:
    mov     edi, edx                    ; edi = edx = src
.move_one_disk:
    add     rsp, 40
    mov     r8d, r12d
    mov     edx, edi
    pop     rbx
    lea     rcx, [prompt_move_one_disk]
    pop     rsi
    pop     rdi
    pop     rbp
    pop     r12
    jmp     printf

%macro str 2
    %2: db %1, 0
%endmacro

section .rdata
    str "How many disks do you want to play with? ", prompt
    str "%u", scan_fmt
    str {"Uh-oh, I couldn't understand that...  No towers of Hanoi for you!", 10}, scan_fail
    str {"Move disk 1 from %d to %d", 10}, prompt_move_one_disk
    str {"Move disk %d from %d to %d", 10}, prompt_move_disk

Sample output

Observations.

Move disk 1 from 1 to 3
Move disk 2 from 1 to 2

I think these messages would be more descriptive (less ambiguous) if you included the word peg:

Move disk 1 from peg 1 to peg 3
Move disk 2 from peg 1 to peg 2

mov     r9d, r12d                   ; update r9d after hanoi call
mov     r8d, esi                    ; update r8d after hanoi cal

I found it confusing to read these comments. Registers R8D and R9D are being setup for the upcoming printf and nothing else. The mention of "update after hanoi call" would suggest they have a more permanent significance which they haven't.

Optimizations.

The .skip_recursive_move operation for when ECX is 1 on input can be much simpler. All registers are already conveniently setup for the tailcall (jmp) to printf.
The program uses the EBP register in a redundant fashion and hence you can shave off a number of instructions. You also no longer need to preserve EBP.

I suggest you take a look at my version below:

hanoi:
    cmp     ecx, 1                      ; if num_disks == 1
    je      .skip_recursive_move

    push    rbx
    push    rsi
    push    rdi
    push    r12

    mov     ebx, ecx                    ; ECX=2+
    mov     esi, edx                    ; esi = edx = src
    mov     edi, r9d                    ; edi = r9d = tmp
    mov     r12d, r8d                   ; r12d = r8d = dst

.move_disk:
    mov     r9d, r12d                   ; dst
    mov     r8d, edi                    ; tmp
    mov     edx, esi                    ; src
    lea     ecx, [rbx - 1]              ; num_disks - 1
    call    hanoi                       ; tmp & dst are swapped for this call

    mov     r9d, r12d                   ; the 'to' peg
    mov     r8d, esi                    ; the 'from' peg
    mov     edx, ebx                    ; num_disks
    lea     rcx, [prompt_move_disk]
    call    printf

    dec     ebx                         ; num_disks - 1
    cmp     ebx, 1                      ; if num_disks == 1
    je      .move_one_disk

    mov     eax, esi
    mov     esi, edi
    mov     edi, eax                    ; swaps tmp and src (esi and edi)
    jmp     .move_disk

.move_one_disk:
    mov     r8d, r12d                   ; the 'to' peg
    mov     edx, edi                    ; the 'from' peg

    pop     r12
    pop     rdi
    pop     rsi
    pop     rbx

.skip_recursive_move:
    lea     rcx, [prompt_move_one_disk]
    jmp     printf

Please notice that setting up the calling registers in a consistent order (RCX, RDX, R8, R9) helps to improve readability. Leaving a blank line further groups things into logical blocks.

One additional optimization comes from exchanging ESIand EDI regardless, thus being able to re-arrange the code to avoid having 2 jump instructions:

    ...
    mov     eax, esi
    mov     esi, edi
    mov     edi, eax                    ; swaps tmp and src (esi and edi)

    dec     ebx                         ; num_disks - 1
    cmp     ebx, 1                      ; if num_disks == 1
    jne     .move_disk

    mov     r8d, r12d                   ; the 'to' peg
    mov     edx, ESI                    ; the 'from' peg
    ...

See that this time ESI gives the 'from' peg!

EDIT (copy-paste version):

;; bits 64
default rel

extern printf, scanf

section .text
    global main
    global hanoi
main:
    sub     rsp, 56
    lea     rcx, [prompt]
    call    printf
    lea     rdx, [rsp+32]
    lea     rcx, [scan_fmt]
    call    scanf
    cmp     eax, 1
    jz      .call_hanoi
    lea     rcx, [scan_fail]
    call    printf
    mov     eax, 1                      ; return 1
.end:
    add     rsp, 56
    ret
.call_hanoi:
    mov     ecx, dword [rsp+32]         ; ecx = num_disks
    mov     r9d, 3                      ; r9d = tmp
    mov     r8d, 2                      ; r8d = dst
    mov     edx, 1                      ; edx = src
    call    hanoi                       ; hanoi(num_disks, src, dst, tmp)
    xor     eax, eax                    ; return 0
    jmp     .end
hanoi:
    cmp     ecx, 1                      ; if num_disks == 1
    je      .skip_recursive_move

    push    rbx
    push    rsi
    push    rdi
    push    r12

    mov     ebx, ecx                    ; ECX=2+
    mov     esi, edx                    ; esi = edx = src
    mov     edi, r9d                    ; edi = r9d = tmp
    mov     r12d, r8d                   ; r12d = r8d = dst

.move_disk:
    mov     r9d, r12d                   ; dst
    mov     r8d, edi                    ; tmp
    mov     edx, esi                    ; src
    lea     ecx, [rbx - 1]              ; num_disks - 1
    call    hanoi                       ; tmp & dst are swapped for this call

    mov     r9d, r12d                   ; the 'to' peg
    mov     r8d, esi                    ; the 'from' peg
    mov     edx, ebx                    ; num_disks
    lea     rcx, [prompt_move_disk]
    sub     rsp, 32                     ;shadow space
    call    printf
    add     rsp, 32

    mov     eax, esi
    mov     esi, edi
    mov     edi, eax                    ; swaps tmp and src (esi and edi)

    dec     ebx                         ; num_disks - 1
    cmp     ebx, 1                      ; if num_disks == 1
    jne     .move_disk

    mov     r8d, r12d                   ; the 'to' peg
    mov     edx, esi                    ; the 'from' peg

    pop     r12
    pop     rdi
    pop     rsi
    pop     rbx

.skip_recursive_move:
    lea     rcx, [prompt_move_one_disk]
    sub     rsp, 32                     ;shadow space
    call    printf
    add     rsp, 32
    ret

%macro str 2
    %2: db %1, 0
%endmacro

section .rdata
    str "How many disks do you want to play with? ", prompt
    str "%u", scan_fmt
    str {"Uh-oh, I couldn't understand that...  No towers of Hanoi for you!", 10}, scan_fail
    str {"Move disk 1 from peg %d to peg %d", 10}, prompt_move_one_disk
    str {"Move disk %d from peg %d to peg %d", 10}, prompt_move_disk


Creating dialogs and modals with AngularJS I wonder whether or not I should improve my current practice on creating/calling dialogs with AngularJS.

Think of a simple information, displayed on demand as a modal window. To create such a thing, I would write a directive, include it to my page and use ng-hide to hide it.

If I want to call it, I would simply change the associated ng-hide value to false from within my main controller.

app.directive('modal',function(){
  return{
    restrict: 'E',
    template: '

',
    // etc
  }
});

app.controller('Ctrl',function($scope){
  $scope.state.modalVisibility = false;

  $scope.changeVisibility = function(){
    $scope.state.modalVisibility = true;
  }
});

Are there any disadvantages or possible improvements on this practice?  Is it a bad idea to have the modal-element "always there" but only displayed on demand?

The disadvantage is the modal directive hanging around for no good reason in the templates, and a tight coupling between the controller and the modal directive.

Far better is using a service to inject this modal into the DOM as needed.
This is the approach that for example the AngularUI team uses in their approach.

This ensures lower coupling (more creative freedom and less technical debt), more easily testable code, and a more intuitive API. Simply inject and call a service, rather than having to make sure the directive is added, and the controller writes to the correct variable, and they share the same scope etc.

The reason you are encouraged to use directives for DOM manipulation is first and foremost convention and testability issues. Using a service in this use-case is an acceptable exception.


Find The Triangular Number With More Than N Divisors My code is a general solution to the following problem from Project Euler #12:

  What is the value of the first triangle number to have over five hundred divisors?

MY CODE

#include <iostream>
int generate_triangular(int n);
int enumerate_divisors(int n);
int main()
{
    std::cout<<"You want to find the first triangular number with less than or more than how many divisors? "<<std::endl;
    int user_input;
    std::cin>>user_input;
    bool result_found = false;
    int test_number = 1;
    int target;
    while(!result_found){
        int factor_count = enumerate_divisors(generate_triangular(test_number));
        if(factor_count < user_input){
            test_number++;
        }else{
            result_found = true;
            target = generate_triangular(test_number);
        }
    }
    std::cout<<"The "<<test_number<<" th triangular number"<<std::endl;
    std::cout<<target<<std::endl;
}
int generate_triangular(int n){
    return (n*(n+1))/2;
}
int enumerate_divisors(int n){
    int divisor_count = 0;
    for(int i = 1;i*i <= n;i++){
        if(n%i == 0){
            divisor_count++;
        }
    }
    return 2*divisor_count;
}

I found my solution to be quite fast for the purposes of the question (it found the first triangular number with over 500 divisors in about .2 seconds). Wondering how I can make it faster, and about any general code suggestions to be made.

Your naming of variables is hard to reason about. When I think about what your program is doing you, I come up with the following sequence of steps:

Take in a target divisor count.
Index through each triangular number n and check if it satisfies the condition.
If it doesn't increment n and repeat.
Once it does print out the results.

Here is how I would restructure your main loop:

std::cout << "You want to find the first triangular number with less than or more than how many divisors? " << std::endl;
int target;
std::cin >> target;
// Indexes the triangle numbers.
int n = 0;
// Keeps track of how many divisors.
int divisors = 0;
do {
    n++;
    divisors = enumerate_divisors(generate_triangular(n));
} while (divisors < target);
std::cout << "The " << n <<" th triangular number" << std::endl;
std::cout << generate_triangular(n) << std::endl;

Also, add some spaces in between your function declarations, it's hard to read when everything is clumped together. But don't over do it (in particular, don't double space everything).


Sending email notifications, each to a different set of subscribers Here is a code which sends email notification on new comments.

<?php

namespace Rib\Src\Apps\ContentNotification\ContentNotificationControllers;

use Rib\Src\Apps\ContentNotification\Email\NewContentNotificationEmail;
use Rib\Src\Services\DbSql;
use Rib\Src\Services\Email;

class JobSendNotificationsCron
{
    public function sendEmails()
    {
        $db = ( new DbSql() )->db()->getConnection();

        # Collect jobs to process
        $jobsToProcess = $db->table( 'content_notifications_jobs' )
                            ->where( 'status', '=', 'active' )
                            ->orderBy( 'last_mailing_time', 'asc' )
                            ->limit( 5 )
                            ->get()->toArray() ?? null;

        # Treat each job indivicually....
        foreach ( $jobsToProcess as $job ) {

            # Get all users that subscribed to be notified about new content in a particular job
            $usersToNotify = $db->table( 'content_notifications_subscribers' )
                                ->where( 'content_id', '=', $job->content_id )
                                ->get()->toArray() ?? null;

            # ... and send notification to each user subscribed to the job
            foreach ( $usersToNotify as $user ) {

                # Send email confirmation email
                Email::getMailer()->send( NewContentNotificationEmail::setContent( $job, $user ) );
            }

            # Update job table. Set job to inactive so it is not treated if no new answer was posted.
            # And set last_mailing_time to also skip jobs that had a mailing before delay was expired.
            $db->table( 'content_notifications_jobs' )->update(
                [
                    'status'            => 'inactive',
                    'last_mailing_time' => time()
                ]
            );
        }
    }
}

I don't like this code because there are sql calls in loop.
I'm thinking of refactoring this code. What would be the best way to refactor it by moving the sql out of the loop ?

Is there any reason for this to be a concrete class/method?

You only seem to be considering happy path.

What happens in DB connection failure occurs?  Have you considered passing the DB connection/object as a dependency to this class or method such that this class/method is guaranteed it gets a connection in a proper state?

Have you considered using a join to get notification/subscriber information in a single query and not have to query in loops?

Email::getMailer()->send(...)

Do you really need to instantiate mailer here every time? Should mailer be a passed dependency?

Do you not need a unique key for performing your update? Right now it looks like you are updating every record on the table.

Do you really want to be updating unix timestamps into the table as opposed to using appropriate MySQL datetime or timestamp fields?


C hack: replace printf to collect output and return complete string by using a line buffer I have this great C program I'd like to embed into an iOS app. One passes command line arguments to it and the results are printed to stdout via printf and fputs - like with all the good old Unix programs.

Now I'd like to just edit main and the print functions to use my own printf function which collects all the output that normally goes to stdout and return it at the end.

I implemented a solution by using a line buffer to collect all the printfs until the newline. And a dynamic char array whereto I copy when an output line is finished.

The charm of this solution is - it's kind of tcl'ish: just throw everything into a text line, and if it's complete, store it. Now do that as long as necessary and return the whole bunch at the end.

C code:

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

// outLineBuffer collects one output line by several calls to tprntf
#define initialSizeOfReturnBuffer 10 // reduced for testing (would be 16*1024)
#define incrSizeOfReturnBuffer 5     // reduced for testing (would be 1024*1024)
#define outLineBufferMaxSize 4095
char outLineBuffer[sizeof(char)*outLineBufferMaxSize] = "";
char *tReturnString;
size_t sizeOfReturnBuffer, curPosOutBuffer = 0, lenOutLine = 0;

With the replacement tprntf for all the original printf and fputs:

// replace printf with this to collect the parts of one output line.
static int tprntf(const char *format, ...)
{
    const size_t maxLen = sizeof(char)*outLineBufferMaxSize;
    va_list arg;
    int done;

    va_start (arg, format);
    done = vsnprintf (&outLineBuffer[lenOutLine], maxLen-lenOutLine, format, arg);
    va_end (arg);
    lenOutLine = strlen(outLineBuffer);

    return done;
}

And the function when we complete one output line (everywhere \n is printed):

// Output line is now complete: copy to return buffer and reset line buffer.
static void tprntNewLine()
{
    size_t newSize;
    long remainingLenOutBuffer;
    char *newOutBuffer;

    remainingLenOutBuffer = sizeOfReturnBuffer-curPosOutBuffer-1;
    lenOutLine = strlen(outLineBuffer)+1; // + newline character (\n)
    remainingLenOutBuffer -= lenOutLine;

    if (remainingLenOutBuffer < 0) {
        newSize = sizeOfReturnBuffer + sizeof(char)*incrSizeOfReturnBuffer;
        if ((newOutBuffer = realloc(tReturnString, newSize)) != 0) {
            tReturnString = newOutBuffer;
            sizeOfReturnBuffer = newSize;
        } else {
            lenOutLine += remainingLenOutBuffer; //just write part that is still available
            remainingLenOutBuffer = 0;
        }
    }

    snprintf(&tReturnString[curPosOutBuffer], lenOutLine+1, "%s\n", outLineBuffer);

    curPosOutBuffer += lenOutLine;
    outLineBuffer[0] = 0;
    lenOutLine = 0;
}

And a little main to test it (without Swift - e.g. plain gcc):

int main(int argc, char *argv[])
{
    int i;
    sizeOfReturnBuffer = initialSizeOfReturnBuffer*sizeof(char);
    if ((tReturnString = malloc(sizeOfReturnBuffer)) == 0) {
        return 1; // "Sorry we are out of memory. Please close other apps and try again!";
    }
    tReturnString[0] = 0;

    for (i = 1; i < argc; i++) {
        tprntf("%s ", argv[i]);
    }
    tprntNewLine();

    tprntf("%s", "ABC\t");
    tprntf("%d", 12);
    tprntNewLine(); // enough space for that ;-)
    tprntf("%s", "DEF\t");
    tprntf("%d", 34);
    tprntNewLine(); // realloc necessary ...
    tprntf("%s", "GHI\t");
    tprntf("%d", 56);
    tprntNewLine(); // again realloc for testing purposes ...
    printf("tReturnString at the end:\n>%s<\n", tReturnString); // contains trailing newline
    return 0;
}

The call from swift will then be as follows (using CStringArray.swift):

let myArgs = CStringArray(["computeIt", "par1", "par2"])
let returnString = mymain(myArgs.numberOfElements, &myArgs.pointers[0])

if let itReturns = String.fromCString(returnString) {
    print(itReturns)
}

freeMemory()

Bug

Your code in tprntNewLine() doesn't guarantee that tReturnString is large enough to hold the output string.

Suppose at entry to the function, these are the values of your variables:

sizeOfReturnBuffer = 10;
curPosOutBuffer    = 0;

And then you get an outLine whose length is 80.  After this piece of code:

if (remainingLenOutBuffer < 0) {
    newSize = sizeOfReturnBuffer + sizeof(char)*incrSizeOfReturnBuffer;
    if ((newOutBuffer = realloc(tReturnString, newSize)) != 0) {
        tReturnString = newOutBuffer;
        sizeOfReturnBuffer = newSize;
    } else {
        lenOutLine += remainingLenOutBuffer; //just write part that is still available
        remainingLenOutBuffer = 0;
    }
}

You will increase the size of your output buffer from 10 to 15 (because you only add incrSizeOfReturnBuffer to the current size).

After that, you will then try to copy 80 bytes from outLine into your 15 byte buffer:

snprintf(&tReturnString[curPosOutBuffer], lenOutLine+1, "%s\n", outLineBuffer);

You need to change your reallocation amount to ensure that it is at least enough to hold the length of the line you are trying to append.  For example:

if (remainingLenOutBuffer < 0) {
    int neededSize = -remainingLenOutBuffer;

    if (neededSize < incrSizeOfReturnBuffer)
        neededSize = incrSizeOfReturnBuffer;
    newSize = sizeOfReturnBuffer + neededSize;
    // ...

Style issues

Typically, constants that are #defined are in capital letters and in snake_case. So I would change:

#define initialSizeOfReturnBuffer 10

to

#define INITIAL_SIZE_OF_RETURN_BUFFER 10

There's no need to use sizeof(char).  It's always 1.


Mixed Drink Calculator in Haskell This is a simple mixed drink calculator written in Haskell.

There are two input files. The drinks file contains a list of simply formatted recipes for mixed drinks:

screwdriver:vodka,orange juice
white russian:vodka,coffee liqueur,cream
greyhound:gin,grapefruit juice
...

The ingredients file contains a line-separated list of presently available ingredients:

vodka
gin
rum
orange juice
cranberry juice
grapefruit juice
...

The calculator parses the drinks file and the ingredients file, determines which mixed drinks can be created with the available ingredients, and pretty-prints the results to screen:

Screwdriver: Vodka, Orange juice
Greyhound: Gin, Grapefruit juice
...

It's a simple program, but I want to improve my code style and make my Haskell more idiomatic.

import Text.ParserCombinators.Parsec
import Data.Char
import Data.List

type Drink = (DrinkName, [Ingredient])
type DrinkName = String
type Ingredient = String

-- parsing the file of drink recipes
drinksFile ∷ GenParser Char st [Drink]
drinksFile = endBy drink (char '\n')

-- parsing each drink recipe
drink ∷ GenParser Char st Drink
drink = do first ← drinkName
           next ← recipe
           return (first, next)

-- parsing the drink name
drinkName ∷ GenParser Char st DrinkName
drinkName = do name ← many (noneOf ":")
               sep ← many (char ':')
               return name

-- parsing the recipe into a list of ingredients
recipe ∷ GenParser Char st [Ingredient]
recipe = sepBy (many (noneOf ",\n")) (char ',')

-- returns true iff the drink can be made with the given ingredients
canMake ∷ [Ingredient] → Drink → Bool
canMake ingredients drink = all (flip elem ingredients) recipe
                            where recipe = snd drink

-- returns the drinks that can be made with the given ingredients
filterByIngredients ∷ [Drink] → [Ingredient] → [Drink]
filterByIngredients drinks ingredients = filter (canMake ingredients) drinks

-- pretty prints a drink with its recipe
printDrink ∷ Drink → String
printDrink (drinkName, recipe) = intercalate " " (map capitalize (words drinkName)) ++
                                ": " ++
                                intercalate ", " (map capitalize recipe)
                                where capitalize (x : xs) = toUpper x : xs

main = do drinks ← readFile "drinks"
          ingredients ← readFile "ingredients"
          case parse drinksFile "drinksFile" drinks of
               Left error → do putStrLn "Error parsing drinks file."
               Right parsedDrinks → mapM_ (putStrLn ∘ printDrink)
                                          (filterByIngredients parsedDrinks
                                                               (lines ingredients))

Good News:

Architecturally, your code looks very well designed. I can't think of any language features that would significantly improve your code's architecture. That leaves only function semantics and readability.

Advise:

This advise isn't specific to idiomatic Haskell but rather general code readability.

Your current main method does a little too much. the contents of main should have a high level of abstraction. Consider abstracting the work done at the end of your main function:

main = do drinks      <- readFile "drinks"
          ingredients <- readFile "ingredients"
          putStrLn $
              case parse drinksFile "drinksFile" drinks of
              Left error         -> "Error parsing drinks file."
              Right parsedDrinks -> showPossibleDrinks parsedDrinks (lines ingredients)

showPossibleDrinks :: [Drink] -> [Ingredient] -> String
showPossibleDrinks drinks ingredients = showDrinks $ filterByIngredients drinks ingredients

showDrinks :: [Drink] -> String
showDrinks =  unlines . map printDrink

Here we have abstracted the process of printing the possible drinks into two sub methods. This provides greater readability and also allows us to change the return type of case from IO to String. Hence the putStrLn outside the case statement.

By changing the argument order of a few functions, we can  move the functions towards pointfree form:

canMake :: [Ingredient] -> Drink -> Bool
canMake ingredients = all (flip elem ingredients) . snd

filterByIngredients :: [Ingredient] -> [Drink] -> [Drink]
filterByIngredients ingredients = filter (canMake ingredients)

main = do drinks      <- readFile "drinks"
          ingredients <- readFile "ingredients"
          putStrLn $
              case parse drinksFile "drinksFile" drinks of
              Left error         -> "Error parsing drinks file."
              Right parsedDrinks -> showPossibleDrinks (lines ingredients) parsedDrinks

showPossibleDrinks :: [Ingredient] -> [Drink] -> String
showPossibleDrinks = showDrinks . (filterByIngredients ingredients)

Given the succinctness of your methods, moving towards pointfree form is quite readable and more idiomatic.

If we import the <$> operator from Control.Applicative after transposing the function's arguments we can make filterByIngredients completely pointfree:

filterByIngredients :: [Ingredient] -> [Drink] -> [Drink]
filterByIngredients =  filter <$> canMake

You can make the following equivalence substitutions:

(flip elem ingredients) ==> (`elem` ingredients)
intercalate " " ==> unwords

Giving you:

canMake :: [Ingredient] -> Drink -> Bool
canMake ingredients = all (`elem` ingredients) . snd

printDrink :: Drink -> String
printDrink (drinkName, recipe) = unwords (map capitalize (words drinkName)) ++
                                ": " ++
                                intercalate ", " (map capitalize recipe)
                                where capitalize (x : xs) = toUpper x : xs

All together we have:

import Control.Applicative ((<$>))
import Data.Char
import Data.List
import Text.ParserCombinators.Parsec

type Drink = (DrinkName, [Ingredient])
type DrinkName = String
type Ingredient = String

-- parsing the file of drink recipes
drinksFile :: GenParser Char st [Drink]
drinksFile =  endBy drink (char '\n')

-- parsing each drink recipe
drink :: GenParser Char st Drink
drink =  do first <- drinkName
            next  <- recipe
            return (first, next)

-- parsing the drink name
drinkName :: GenParser Char st DrinkName
drinkName =  do name <- many (noneOf ":")
                sep  <- many (char ':')
                return name

-- parsing the recipe into a list of ingredients
recipe :: GenParser Char st [Ingredient]
recipe =  sepBy (many (noneOf ",\n")) (char ',')

canMake :: [Ingredient] -> Drink -> Bool
canMake ingredients = all (`elem` ingredients) . snd

filterbyIngredients :: [Ingredient] -> [Drink] -> [Drink]
filterbyIngredients = filter <$> canMake

-- pretty prints a drink with its recipe
printDrink :: Drink -> String
printDrink (drinkName, recipe) = unwords (map capitalize (words drinkName)) ++
                                ": " ++
                                intercalate ", " (map capitalize recipe)
                                where capitalize (x : xs) = toUpper x : xs

showPossibleDrinks :: [Ingredient] -> [Drink] -> String
showPossibleDrinks ingredients = showDrinks . filterbyIngredients ingredients

showDrinks :: [Drink] -> String
showDrinks =  unlines . map printDrink

main = do drinks      <- readFile "drinks"
          ingredients <- readFile "ingredients"
          putStrLn $
              case parse drinksFile "drinksFile" drinks of
              Left error         -> "Error parsing drinks file."
              Right parsedDrinks -> showPossibleDrinks (lines ingredients) parsedDrinks


Python - Game stat calculator So this is the code to a calculator I made for a game. Essentially what the calculator does is, it calculates the cost of buying the next stat.
So I wrote the code just for the Human race and the strength stat. From the way I see it, I will have to do for each race the same code 3 times for each stat.
I was hoping there would be a shorter way around this like
instead of human.strength I would like it to be race.strength where race = user_race.
Thanks
class race:
    """The different races in the calculator"""
    def __init__(self, race, strength, agility, health, strength_cost, agility_cost, health_cost):
        self.race = race
        self.strength = strength
        self.agility = agility
        self.health = health
        self.strength_cost = strength_cost
        self.agility_cost = agility_cost
        self.health_cost = health_cost

human = race('Human', 15, 17, 18, 5, 3, 4)
elf = race('Elf', 11, 21, 14, 4, 3, 5)
giant = race('Giant', 25, 11, 27, 4, 8, 3)

print("Human, Giant, Elf")
user_race = str(input("Enter your race:")).lower()
print("Strength, Agility, Health")
user_stat = str(input("Enter your desired stat:")).lower()
user_present_stat_value = int(input("Enter your present stat value:"))
user_desired_stat_value = int(input("Enter your desired stat value:"))

if user_race == 'human' and user_stat == 'strength':
    human_strength_present_statdif = (user_present_stat_value - human.strength) # difference of present stat with respect of base stat

    human_strength_desired_statdif = (user_desired_stat_value - human.strength) #difference of desired stat with respect of base stat

    human_strength_present_stat_cost = (human.strength_cost + (human_strength_present_statdif - 1) * human.strength_cost) #The cost of present stat stat

    human_strength_total_present_cost = ((human_strength_present_statdif / 2) * (human.strength_cost + human_strength_present_stat_cost)) # The total cost from base stat to present stat

    human_strength_desired_stat_cost = (human.strength_cost + (human_strength_desired_statdif - 1) * human.strength_cost) #The cost of desired stat

    human_strength_total_desired_cost = ((human_strength_desired_statdif / 2) * (human.strength_cost + human_strength_desired_stat_cost)) # The total cost base stat to desired stat

    human_strength_net_cost = (human_strength_total_desired_cost - human_strength_total_present_cost) # The Net cost from the difference of Total desired stat and Total present stat

    print("Net cost: " + str(human_strength_net_cost))
```

If you are just trying to make an interactive calculator, the classes, etc. are not needed.
First, make a simple table that lets you look up the stats based on race. Keep it easy for a human (like you) to edit it, make changes, add new races or stats, etc.
keys = "base_strength base_agility base_health strength_cost agility_cost health_cost".split()

traits = [
    #        base     base    base  strength agility health
    #race  strength agility  health   cost     cost   cost
    "human    15       17      18      5        3      4",
    "elf      11       21      14      4        3      5",
    "giant    25       11      27      4        8      3",
]

It's just a list of strings. Now transform it into a format that makes it easy to use in a program.  We'll turn it into a dict of dicts so we can look up values using something like: stat["elf"]["base_agility"].  Here's the code:
stats = {}

for row in traits:
    row = row.strip().split()
    stats[row[0]] = dict(zip(keys, map(int, row[1:])))

Now your code that calculates the cost of changing strength for a human, can be turned into a generic function that works for any race or stat:
def calc_change_cost(race, stat_name, present_value, desired_value):
    base_value = stats[race][f"base_{stat_name}"]
    stat_cost = stats[race][f"{stat_name}_cost"]

    present_statdif = present_value - base_value
    present_stat_cost = stat_cost + (present_statdif - 1) * stat_cost
    total_present_cost = (present_statdif / 2) * (stat_cost + present_stat_cost)

    desired_statdif = desired_value - base_value
    desired_stat_cost = stat_cost + (desired_statdif - 1) * stat_cost
    total_desired_cost = (desired_statdif / 2) * (stat_cost + desired_stat_cost)

    net_cost = total_desired_cost - total_present_cost

    return net_cost

You'll notice the repeated code for calculating total_present_cost and total_desired_cost.  Those lines could be refactored into another function (an exercise for the reader).
Now, the main program just collects the user's inputs, calls the function above, and prints the results:
user_race = str(input("Enter your race (Human, Giant, Elf):")).lower()
user_stat = str(input("Enter your desired stat (Strength, Agility, Health):")).lower()
present_value = int(input("Enter your present stat value:"))
desired_value = int(input("Enter your desired stat value:"))

net_cost = calc_change_cost(user_race, user_stat, present_value, desired_value)
print(f"Net cost to change {user_race} {user_stat} from {present_value} to {desired_value}: {net_cost}")


Find the longest word in a string I have written code in the language Go. I wonder if anyone can do it in a very simple and effective way, because every language has its own way to solve this problem. My experience is in JavaScript. Please guide me how I can improve this solution. Thank you in advance.

What I'm doing here:

  Input  -

"I love dogs"
"fun&!! time"

  Output :-

love
time

  Here I return the longest word in the string. If there are two or more
  words that are of same length, return the first word from the string
  with that length.

package main

    import (
        "fmt"
        "regexp"
    )

    func main() {
        re := regexp.MustCompile(`[A-Za-z]+|[*?()$&.,!]`)
        matches := re.FindAllString("fun&!! time", -1)
        var count int
        for i := 0; i < len(matches); i++ {

            if (i+1)%len(matches) > 0 {

                if len(matches[(i+1)%len(matches)]) > len(matches[i]) {
                    count = (i + 1) % len(matches)

                }

            }

        }
        fmt.Println(matches[count])
    }

The first thing to do is to incorporate some basic notions of organization and design into your program, for example, a maxWord function.

package main

import (
    "fmt"
    "regexp"
)

var re = regexp.MustCompile(`[A-Za-z]+|[*?()$&.,!]`)

func maxWord(s string) string {
    matches := re.FindAllString(s, -1)
    var count int
    for i := 0; i < len(matches); i++ {
        if (i+1)%len(matches) > 0 {
            if len(matches[(i+1)%len(matches)]) > len(matches[i]) {
                count = (i + 1) % len(matches)
            }
        }
    }
    return matches[count]
}

func main() {
    for _, s := range []string{
        "I love dogs",
        "fun&!! time",
        loremipsum,
    } {
        fmt.Printf("%q\n", maxWord(s))
    }
}

var loremipsum = `
Lorem ipsum dolor sit amet, consectetur adipiscing elit,
sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
Ut enim ad minim veniam,
quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident,
sunt in culpa qui officia deserunt mollit anim id est laborum.
`

Output:

"love"
"time"
"laborum"

Test your code. Your result for the loremipsum text doesn't look right.

Consider writing a simple, efficient word parser. For example,

package main

import (
    "fmt"
    "unicode"
)

func maxWord(s string) string {
    var word string
    inWord := false
    j, k := 0, 0
    for i, r := range s {
        if !unicode.IsLetter(r) {
            if inWord {
                k = i
                if k-j > len(word) {
                    word = s[j:k]
                }
            }
            inWord = false
        } else if !inWord {
            inWord = true
            j = i
        }
    }
    if inWord {
        k = len(s)
        if k-j > len(word) {
            word = s[j:k]
        }
    }
    return word
}

func main() {
    for _, s := range []string{
        "I love dogs",
        "fun&!! time",
        loremipsum,
    } {
        fmt.Printf("%q\n", maxWord(s))
    }
}

var loremipsum = `
Lorem ipsum dolor sit amet, consectetur adipiscing elit,
sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.
Ut enim ad minim veniam,
quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.
Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.
Excepteur sint occaecat cupidatat non proident,
sunt in culpa qui officia deserunt mollit anim id est laborum.
`

Output:

"love"
"time"
"reprehenderit"

Go programmers often write reasonably efficient programs. The Go standard library testing package provides a benchmark facility.

For example,

$ go test maxword_test.go -bench=. -benchmem
BenchmarkPeterSO-4    24316906      47.7 ns/op      0 B/op     0 allocs/op
BenchmarkMannu-4        557594    2022 ns/op      448 B/op    10 allocs/op

maxword_test.go:

package main

import (
    "regexp"
    "testing"
    "unicode"
)

func maxWordP(s string) string {
    var word string
    inWord := false
    j, k := 0, 0
    for i, r := range s {
        if !unicode.IsLetter(r) {
            if inWord {
                k = i
                if k-j > len(word) {
                    word = s[j:k]
                }
            }
            inWord = false
        } else if !inWord {
            inWord = true
            j = i
        }
    }
    if inWord {
        k = len(s)
        if k-j > len(word) {
            word = s[j:k]
        }
    }
    return word
}

func BenchmarkPeterSO(b *testing.B) {
    for N := 0; N < b.N; N++ {
        for _, s := range []string{"I love dogs", "fun&!! time"} {
            maxWordP(s)
        }
    }
}

var re = regexp.MustCompile(`[A-Za-z]+|[*?()$&.,!]`)

func maxWordM(s string) string {
    matches := re.FindAllString(s, -1)
    var count int
    for i := 0; i < len(matches); i++ {
        if (i+1)%len(matches) > 0 {
            if len(matches[(i+1)%len(matches)]) > len(matches[i]) {
                count = (i + 1) % len(matches)
            }
        }
    }
    return matches[count]
}

func BenchmarkMannu(b *testing.B) {
    for N := 0; N < b.N; N++ {
        for _, s := range []string{"I love dogs", "fun&!! time"} {
            maxWordM(s)
        }
    }
}

var benchTexts = []string{"I love dogs", "fun&!! time"}


Google oAuth Angular service I wrote a module with 3 services that execute the oauth (and set the api key) for the google api and return the token object if the promise is positive.

Here is the whole working module:

angular.module('gapiOAuth', [])
.controller('gapiOAuthController', ['$scope', function($scope){
}])

.factory('gapiCredentialsSrvc', function(){
    return {
        gapiClientId: 'xxx',
        gapiScopeUrl: ['https://www.googleapis.com/auth/userinfo.email','https://www.googleapis.com/auth/blogger'],
        gapiKey: 'xxx'
    }
})

.factory('gapiGetTokenSrvc', ['$q','gapiCredentialsSrvc', function($q, gapiCredentialsSrvc){
    return function (){
        var q = $q.defer();
        angular.element(document).ready(function(){
            gapi.auth.authorize({client_id: gapiCredentialsSrvc.gapiClientId, scope: gapiCredentialsSrvc.gapiScopeUrl, immediate: true}, function(response){
                if(response && !response.error){
                    q.resolve(response);
                } else {
                    q.reject({});
                }
            });
        });
        return q.promise;
    };
}])

.factory('gapiLogInSrvc',['$q','gapiCredentialsSrvc', function($q, gapiCredentialsSrvc){
    return function(){
        var q = $q.defer();
        angular.element(document).ready(function(){
            gapi.auth.authorize({client_id: gapiCredentialsSrvc.gapiClientId, scope: gapiCredentialsSrvc.gapiScopeUrl, immediate: false}, function(response){
                if(response && !response.error){
                    q.resolve(response);
                }else{
                    q.reject(response.error);
                }
            });
        });
        return q.promise;
    };
}])

.factory('gapiSetApiKeySrvc',['gapiCredentialsSrvc', function(gapiCredentialsSrvc){
    return function(){
        angular.element(document).ready(function(){
            gapi.client.setApiKey(gapiCredentialsSrvc.gapiKey);
            return true;
        });
    };
}])

Do you think there will be security issues if I put this code I made onto the client side?

How can I improve it?

Do you think there will be security issues if I put this code I made onto the client side?

Yes. You should not expose your Google API key to other users - it should be hidden away on your server. You should expose an end point on an API server that will run the Google OAuth actions. This is known as the three-legged or server-side OAuth flow.

The security reasons mainly come down to the fact that if someone knows your API key then they can masquerade as you. This is usually solved by a shared secret (or just a private key), but the Google OAuth flow doesn't appear to provide this.

You can see here for Google's own resource on best practises for API keys.


Improve mixin that outputs page-specific CSS As part of this answer, I created a mixin to generate some page-specific CSS for me.

I need to style an element based on a class I assign to the html-element. I do this for four different classes on the root-element, which gives me four similar CSS rules.

I'm looking for suggestions for the mixin itself. This is not limited to its function, also naming and the general idea may have so room for improvements.

Usage:

Pass the $element you want to style...
...and the $property (needs to hold a color value) you want to style i with
optionally, pass true as a third argument, if the light color variation should be used

@include themify(".page-title", background-color, true);

Example Output:

.page--blog .page-title {
    /* $blog-color-light */
    background-color: #6fbf00;
}

.page--portfolio .page-title {
    /* $portfolio-color-light */
    background-color: #ffaa10;
}

.page--profil .page-title {
    /* $profil-color-light */
    background-color: #6d91da;
}

.page--impressum .page-title {
    /* $impressum-color-light */
    background-color: #e04345;
}

Mixin:

@mixin themify($element, $property, $light-colors: false) {
    $pages: null;
    @if $light-colors == true {
        $pages: blog      $blog-color-light,
                portfolio $portfolio-color-light,
                profil    $profil-color-light,
                impressum $impressum-color-light;
    } @else {
        $pages: blog      $blog-color,
                portfolio $portfolio-color,
                profil    $profil-color,
                impressum $impressum-color;
    }

    @each $page in $pages {
        .page--#{nth($page, 1)} #{$element} {
            #{$property}: nth($page, 2);
        }
    }
}

This uses mappings, which are part of Sass 3.3. You can do the same thing without mappings, it's just not as pretty. In addition to allowing multiple selectors, it allows for more options than just light/dark colors (eg. background images, etc):

$themes:
    ( blog:
        ( default: red
        , light: lighten(red, 20%)
        )
    , portfolio:
        ( default: orange
        , light: lighten(orange, 20%)
        )
    , profile:
        ( default: green
        , light: lighten(green, 20%)
        )
    , impressum:
        ( default: blue
        , light: lighten(blue, 20%)
        )
    );

@mixin themify($property, $color: default) {
    @each $theme, $colors in $themes {
        .page--#{$theme} & {
            #{$property}: map-get($colors, $color);
        }
    }
}

.foo, .bar {
    @include themify(background-color);
    @include themify(color, light);
}

Output:

/* line 27, ../sass/test.scss */
.page--blog .foo, .page--blog .bar {
  background-color: red;
}
/* line 27, ../sass/test.scss */
.page--portfolio .foo, .page--portfolio .bar {
  background-color: orange;
}
/* line 27, ../sass/test.scss */
.page--profile .foo, .page--profile .bar {
  background-color: green;
}
/* line 27, ../sass/test.scss */
.page--impressum .foo, .page--impressum .bar {
  background-color: blue;
}
/* line 27, ../sass/test.scss */
.page--blog .foo, .page--blog .bar {
  color: #ff6666;
}
/* line 27, ../sass/test.scss */
.page--portfolio .foo, .page--portfolio .bar {
  color: #ffc966;
}
/* line 27, ../sass/test.scss */
.page--profile .foo, .page--profile .bar {
  color: #00e600;
}
/* line 27, ../sass/test.scss */
.page--impressum .foo, .page--impressum .bar {
  color: #6666ff;
}

However, as you can tell from the output, it generates fairly inefficient CSS if you're using more than one property per selector.  You can deal with this by manipulating global variables (again, 3.3 syntax):

$colors: null; // private, don't touch

$themes:
    ( blog:
        ( base: red
        , light: lighten(red, 20%)
        )
    , portfolio:
        ( base: orange
        , light: lighten(orange, 20%)
        )
    , profile:
        ( base: green
        , light: lighten(green, 20%)
        )
    , impressum:
        ( base: blue
        , light: lighten(blue, 20%)
        )
    );

@mixin themify {
    @each $theme, $c in $themes {
        .page--#{$theme} {
            $colors: $c !global;

            @content;
        }
    }
}

@function theme($color: base) {
    @return map-get($colors, $color);
}

@include themify {
    .foo, .bar {
        background-color: theme(base);
        color: theme(light);
    }
}

Output:

/* line 42, ../sass/test.scss */
.page--blog .foo, .page--blog .bar {
  background-color: red;
  color: #ff6666;
}

/* line 42, ../sass/test.scss */
.page--portfolio .foo, .page--portfolio .bar {
  background-color: orange;
  color: #ffc966;
}

/* line 42, ../sass/test.scss */
.page--profile .foo, .page--profile .bar {
  background-color: green;
  color: #00e600;
}

/* line 42, ../sass/test.scss */
.page--impressum .foo, .page--impressum .bar {
  background-color: blue;
  color: #6666ff;
}


One-way hash using ASCII Assume the built-in function Ascii(c), where c is a one character
  string, will return an integer containing the ASCII value of that
  character. Write a function that accepts a string, uses Ascii(c) and
  additional processing to produce a one way hash value from the string,
  and returns the hash value as a string. You may assume the initial
  string is not empty and contains several characters.

//one way hash using ascii
static string hashViaAscii(String S)
{
    int hashVal = 0;

    for (int k = 0; k < S.Length; k++)
        hashVal += Ascii(S[k]);  //gets Ascii value, add them all

    return ((hashVal % HASH_TABLE_SIZE).ToString());  //return sum % table size as string
}

I don't think is a good approach. How can this be improved?

I'm confused by the phrase “one way hash value” and so is probably whoever specified this requirement. In cryptography, a one-way hash function has a well-defined meaning.

  Let \$\Sigma\$ be an alphabet, for example, \$\Sigma=\{\mathtt{0}, \mathtt{1}\}\$. A family of functions \$(f_k)_{k\in\mathbb{N}}\$ with
  $$f_k: \Sigma^k \times \Sigma^* \to \Sigma^k$$
  is a family of one-way hash functions if and only if for each \$k\in\mathbb{N}\$, the \$f_k\$ is computable by a polynomial-time algorithm and for every probabilistic polynomial-time algorithm \$\mathcal{A}\$, given random \$K\in\Sigma^k\$ and \$h\in\Sigma^k\$, the output \$m^*=\mathcal{A}(K,h)\$ will satisfy \$f_k(K,m^*)=h\$ only with probability negligible in \$k\$.

  \$K\$ is called key or salt. It should be chosen randomly but need not be kept secret.

It is unknown whether such functions exist at all but there are some functions for which it hasn't been proved yet that they are not one-way functions.

If your function should compute such a cryptographic hash, I recommend that you look into one of the existing cryptographic hash function even though I doubt that you're supposed to implement one of these. (Although returning the result as a string would be meaningful in this context because for \$k>64\$, you usually cannot return the hash as an integer.) MD5 and SHA1 are two classical examples of cryptographic hash functions that are reasonably straight-forward to implement. The collision resistance (which is  a stronger requirement than the one-way property) of both of them is known to be broken in theory. (Both are also only specified for fixed \$k\$ which means that the above definition doesn't really apply.)

Since I see the word “hash table” in your code, I suspect that instead of a cryptographic hash function, they wanted to see a “simple” hash function that is fast to compute and has low probability of collisions but is not collision-resistant against malicious attackers. Again, have a look at algorithms other people already invented.

The Fowler-Noll-Vo hash function gives good hashes and is easy to implement. It can also be parametrized on the length of the result. I would recommend the FNV-1a variant as your go-to hash function unless you have reasons to chose differently.
Jenkins' one-at-a-time hash function is equally easy to implement and will probably be faster because it only uses additions and bit-wise operations as opposed to multiplications.
Weinberger's PJW hash function is known for its use in the UNIX ELF format and for being mentioned in the Red Dragon Book.
The lose-lose hash function is the one you've actually implemented. It's name should give you a hint. To your rescue, it was once used as an example by Kernighan and Ritchie.

There are numerous other hash functions and the literature / internet is full of comparisons between them. As you can imagine, while there are clearly better and less good functions available, there is also a lot of personal preference involved in deciding which function to use.

Even if you're planning to use the hash as index for a hash table, don't take the hash modulo the table size in the hash function itself. This will needlessly limit its usefulness. Whoever uses the function should be responsible for wrapping the computed hash into the table size.


Converting string to a set of integers So I get a user input of a lot of numbers in a single line like this

100 100 200 300 300 300 400 500 500 700.

I want to store the distinct ones in a list.
A simple approach is to do this:

nums = input().split(' ')
myList = list(set([(int(i) for i in nums]))

My question is, is this a faster/better way:

nums = input().split(' ')
setList = set()
myList = []
for i in nums:
    if int(i) not in setList:
        myList.append(int(i))
        setList.add(int(i))

No, it is not. The reason is that list comprehensions are in general as fast as or faster than the explicit loop, because they might be performed in C, while the explicit loop will always execute in Python. Also, the set constructor already takes care of the i not in setList part and you need to traverse the whole list anyways.

Here is a shorter one-liner:

numbers = list(set(map(int, input().split())))

I used map instead of a list comprehension, but you could also use a generator expression:

numbers = list(set(int(i) for i in input().split()))

Note that str.split by default splits on whitespace, so there is no need to specify split(' '), unless you want a different result for strings like 100\n200(newline) or 100  200 (double space).

As a timing comparisons, here are the four proposals as functions taking an input string and returning the desired list:

def op1(s):
    nums = s.split(' ')
    return list(set([int(i) for i in nums]))

def op2(s):
    nums = s.split(' ')
    setList = set()
    myList = []
    for i in nums:
        if int(i) not in setList:
            myList.append(int(i))
            setList.add(int(i))
    return myList

def graipher1(s):
    return list(set(map(int, s.split())))

def graipher2(s):
    return list(set(int(i) for i in s.split()))

And this is how I generated a bunch of random inputs of increasing size:

import random
inputs = [" ".join(map(str, [random.randrange(1000) for _ in range(i)])) for i in range(1, 2000, 50)]

With this, I ran the four functions to get this timing graph as a function of the number of numbers in the input:

Note that your first function performs almost as good as a straight set comprehension. Your alternative proposal is always the worst of all possibilities. The version with map is the fastest of the approaches I tried.

(Note that my timings were done using Python 2.7, so your mileage may vary, but I am confident that your second proposal will still loose.)


Chain of Responsibility Design pattern in C++ I am learning design patterns from Design pattern for dummies. From that book I have written a chain of responsibility pattern with two programs. Can someone review and let me know which one is proper chain of responsibility design pattern?

A)

#include<iostream>

using std::cout;
using std::endl;

const int FRONT_END_HELP = 0;
const int INTERMEDIATE_LAYER_HELP = 1;
const int GENERAL_HELP = 2;

class HelpInterface
{
public:
    virtual void getHelp(int iHelpConstant) = 0;
};

class FrontEnd : public HelpInterface {
    HelpInterface * Successor;

public:
        FrontEnd(HelpInterface * s){ Successor = s;}
        void getHelp(int iHelpConstant);
};

void FrontEnd::getHelp(int iHelpConstant)
{
    if (FRONT_END_HELP == iHelpConstant)    {
        cout<<"This is the front end, don't you like it ?\n"<<endl;
    } else {
        Successor->getHelp(iHelpConstant);
    }
}

class IntermediateLayer : public HelpInterface{
      HelpInterface * Successor;
public:
    IntermediateLayer(HelpInterface * s) {Successor = s;}
    void getHelp(int iHelpConstant);
};

void IntermediateLayer::getHelp(int iHelpConstant)
{
    if (INTERMEDIATE_LAYER_HELP == iHelpConstant ) {
        cout<<"This is intermediate layer, nice eh?\n"<<endl;
    } else {
        Successor->getHelp(iHelpConstant);
    }
}

class Application : public HelpInterface{

public:
    Application() {}
    void getHelp(int iHelpConstant) {
        cout<<"This the application help\n"<<endl;
    }
};

int main()
{
    Application *app = new Application();
    IntermediateLayer *ilayer = new IntermediateLayer(app);
    FrontEnd * fend = new FrontEnd(ilayer);

    fend->getHelp(GENERAL_HELP);

    fend->getHelp(INTERMEDIATE_LAYER_HELP);

    system("Pause");

}

B)

#include<iostream>

using std::cout;
using std::endl;

const int FRONT_END_HELP = 0;
const int INTERMEDIATE_LAYER_HELP = 1;
const int GENERAL_HELP = 2;

class HelpInterface
{
protected :
    HelpInterface * Successor;
public:
    virtual void getHelp(int iHelpConstant) = 0;
};

class FrontEnd : public HelpInterface {

public:
        FrontEnd(HelpInterface * s){ Successor = s;}
        FrontEnd(){ Successor = NULL;}
        void getHelp(int iHelpConstant);
};

void FrontEnd::getHelp(int iHelpConstant)
{
    if (FRONT_END_HELP == iHelpConstant)    {
        cout<<"This is the front end, don't you like it ?\n"<<endl;
    } else {
        if (Successor != NULL){
            Successor->getHelp(iHelpConstant);
        } else {
            cout<<"No successor\n"<<endl;
        }
    }
}

class IntermediateLayer : public HelpInterface {
public:
    IntermediateLayer(HelpInterface * s) {Successor = s;}
    IntermediateLayer(){ Successor = NULL;}
    void getHelp(int iHelpConstant);
};

void IntermediateLayer::getHelp(int iHelpConstant)
{
    if (INTERMEDIATE_LAYER_HELP == iHelpConstant ) {
        cout<<"This is intermediate layer, nice eh?\n"<<endl;
    } else {
        if (Successor != NULL){
            Successor->getHelp(iHelpConstant);
        } else {
            cout<<"No successor\n"<<endl;
        }
    }
}

class Application : public HelpInterface{

public:
    Application() {Successor = NULL;}
    void getHelp(int iHelpConstant) {
        cout<<"This the application help\n"<<endl;
    }
};

int main()
{
    Application *app = new Application();
    IntermediateLayer *ilayer = new IntermediateLayer(app);
    FrontEnd * fend = new FrontEnd(ilayer);

    fend->getHelp(GENERAL_HELP);

    fend->getHelp(INTERMEDIATE_LAYER_HELP);

    system("Pause");
}

Design Pattern Comments
It works as a chain of responsibility (and the point of patterns is that they are not exact and are chained depending on exact need).
BUT saying that I think you could have done it better in this situation.

Your descent of the the chain is manual and repeated in any every class.
This should be automated in the base class. Each derived class should simply perform the action that you wan them too.
This will remove programmer error when a new developer creates a chaining task.
Your creation of the chain elements is not consistent. It would be easier to have a setTask() method then force every new task to join the chain rather than force every class to have a boiler plate constructor (that can be done incorrectly).
You do not always want to chain all the way to the end.
On of the handlers should be able to say I have done the final processing stop searching the chain.
No memory management (but that was covered by @Mantosh Jumar).
Also note using smart pointers helps you with another design pattern (Separation of Concerns).

I would have started like this:
class HelpInterface
{
        // Private virtual function.
        // You don't want it called directly as it just does the action
        // None of the chaining.
        virtual bool getHelp(int iHelpConstant) = 0;
        // Note it returns bool.
        // true indicates I have done it all.
        // Do not activate any more tasks in the chain.
        // false indicates try the next item in the chain.

        // Always have a next pointer in all HelpInterface
        std::unique_ptr<HelpInterface>   next;

    public:

        // Need  virtual destructor to make sure memory management is done
        // done correctly. As elements are destroyed via a pointer to the
        // base class.
        virtual ~HelpInterface() {}

        // Chain the getHelp.
        // But stop if it returns true (or we are the last element
        // in the chain.
        void activateHelp(int iHelpConstant)
        {
            if (!getHelp(iHelpConstant) && next)
            {
                next->activateHelp(iHelpConstant);
            }
        }

        void setNextTask(std::unique_ptr<HelpInterface>&& nextElement)
        {
            next = std::move(nextElement);
        }

};

Now you can create the chain like this:
int main()
{
    std::unique_ptr<HelpInterface> app    = std::make_unique<Application>();
    std::unique_ptr<HelpInterface> ilayer = std::make_unique<IntermediateLayer>();
    std::unique_ptr<HelpInterface> fend   = std::make_unique<FrontEnd>();

    iLayer->setNextTask(std::move(app));
    fend->setNextTask(std::move(iLayer));

    fend->getHelp(GENERAL_HELP);
    fend->getHelp(INTERMEDIATE_LAYER_HELP);
}

General Coding comments
A tiny bit lazy here.
using std::cout;
using std::endl;

But these are used so often I will not complain. I would have preferred it if you had not polluted the global namespace for everybody (in this file).
All caps identifier are by tradition reserved for macros. Macros have no regard for scope so you are putting yourself in a dangerous position using all caps for constants.
const int FRONT_END_HELP = 0;
const int INTERMEDIATE_LAYER_HELP = 1;
const int GENERAL_HELP = 2;

Also could have used an enumerate. I personally would have done.
enum HelperTypes { FrontEndHelp, IntermediateLayerHelp, GeneralHelp};

Doing it this way, makes the compiler generate the actual numbers (so there is no chance that somebody accidentally puts two identical numbers into the list). Also if you change your getHelp() signature to use the enum people can not accidentally pass an integer, as the compiler will spot that and force them to use the correct type.
A base class with virtual members. Must usually have a virtual destructor.
class HelpInterface
{
protected :
    HelpInterface * Successor;
public:
    virtual void getHelp(int iHelpConstant) = 0;
};

Also in C++ never us RAW pointers (there are some exceptions). But RAW pointers do not convey ownership. If you don't know the ownership then you don't know who is responsible for deleting the object. One of the most important concepts in C++ (over C) is the concept of ownership (RAII being more important). We should always know who owns a pointer so we know who is going to clean it up. Smart pointers are the usuall technique of indicating ownership (or using references to indicate the passing of a reference with no ownership transfer).
The * is part of the type. So put the * by the type when declaring pointers.
 HelpInterface*     successor;   // object (anything you can get an address of)
                                 // this includes functions. Should have an
                                 // initial lower case letter.

 // User defined types have an initial uppercase letter.
 // This helps you distinguish Types from objects.

You use at least two different styles of bracing:
class HelpInterface
{                                       // Brace on the next line.

class FrontEnd : public HelpInterface { // Brace on the same line.

Pick one and be consistent.
Note: You can use different styles in different places.
class X
{                 // Style for classes.

    int func() {  // Style for code.

    }
};

Personally I always put the '{' on a new line by itself. Others find this wastes vertical space. I buy a big monitor and keep my functions relatively short.
Use default parameters to avoid this situation.
        FrontEnd(HelpInterface * s){ Successor = s;}
        FrontEnd(){ Successor = NULL;}

Can be replaced with:
        FrontEnd(HelpInterface* successor = nullptr)
            : successor(successor) // Note:  member and parameter name the same.
        {}

You have plenty of space between operators here:
    if (FRONT_END_HELP == iHelpConstant)    {

But here they are all packed together. Space is your frined use it.
        cout<<"This is the front end, don't you like it ?\n"<<endl;

Don't use std::endl unless you want to force a flush. This is rarely the case. std::cin and std::cout are linked. If user input is required the cout is automatically flushed so the user can see any questions (or output). Excessive flushing severly degrades performance.
The most hidious of things ever:
    system("Pause");

Stop using it.
Try.
    std::cout << "Hit Enter to continue\n";
    std::string line;
    std::getline(std::cin, line);

This is portable.


Results Reporting / Checking with Multiple Results in For Loop hoping to be schooled a little bit here in showing "no results".
This is a working snippet, but I'd like to know if there is a more elegant way to show errors when comparing multiple values in an IF statement within a ForLoop... I could have 10 results with only 2 matching my if statement:

if($value["IsArchived"] == false && !preg_match("/^123 ABC/", $value["ProjectName"]))

Is there better way than capturing a count variable $totalToShow == 0 to show my "No Results" in a new IF statement or is that ok?

$obj = drupal_json_decode($result);
//used to show NO RECORDS or not... probably a more elegant way here?!
$totalToShow = 0;
//loop through the json data and add it to the $output array.
//NOTE: not checking if any are empty
$output .= '
';
//check if the obj is empty, if so, no records to display...
if (!empty((array) $obj)) {
  foreach($obj as $key=>$value){
    if($value["IsArchived"] == false && !preg_match("/^123 ABC/", $value["ProjectName"])){
        $totalToShow++;
        //output project name link to project # and append start/end date after link.
        $output .= '
' . $value["ProjectName"] . ' ('. _abc_date($value["CommentStart"]) . " - " .  _abc_date($value["CommentEnd"]).') ';
        if($lrnmore != ""){
            $output .= ' | '. $lrnmore .'';
        }
        $output .= '| View Comments
';
        $output .= '

' . $value["Description"] . '

';
        $output .= "
";
    }
  }
  if($totalToShow == 0){
    $output .= '
No Records to Display
';
  }

In case there is any chance to use a template engine, then you should prepare your data first

$obj = drupal_json_decode($result);
$output = [];
foreach($obj as $row){
    if(!$value["IsArchived"] && !preg_match("/^123 ABC/", $value["ProjectName"])){
        $row['CommentStart'] = _abc_date($value["CommentStart"]);
        $row['CommentEnd'] = _abc_date($value["CommentEnd"]);
        $output[] = $row;
    }
}

and then output it (an exampe is using Twig template engine but the logic would be the same even with raw PHP)

{% for value in output %}

        {{ value["ProjectName"]}}
        ({{ value["CommentStart"] }} - {{ value["CommentEnd"] }})
        {% if lrnmore %}
            |
                 {{ lrnmore }}

        {% endif %}
        |
            View Comments

 {{ value["Description"] }}

{% endfor %}
{% if not output %}

No Records to Display

{% endif %}

this way you will have a clear separation between the business logic and display logic and as a bonus you will have HTML which is readable and maintainable.


Scraping information from a webpage not knowing it's last page number I've written some code in vba to scrape names and phone numbers from a webpage that has spread across some pages I don't wish to know of. The main interesting thing with this scraper is that It only needs to know the first page number then it traverse across all the pages and fetch the information I've mentioned above. I tried to make it error-free. Here is what I did:

Sub Yellowpage_Parser()

Const mlink = "https://www.yellowpages.com/search?search_terms=pizza&geo_location_terms=San%20Francisco%2C%20CA&page="
Dim http As New XMLHTTP60, html As New HTMLDocument
Dim post As HTMLHtmlElement

Do
    y = y + 1
    With http
        .Open "GET", mlink & y, False
        .send
        html.body.innerHTML = .responseText
    End With

        For Each post In html.getElementsByClassName("info")
            With post.getElementsByClassName("n")(0).getElementsByTagName("span")
                If .Length Then x = x + 1: Cells(x, 1) = .item(0).innerText
            End With

            With post.getElementsByClassName("phones phone primary")
                If .Length Then Cells(x, 2) = .item(0).innerText
            End With
        Next post

Loop While InStr(http.responseText, "next ajax-page")

    Set html = Nothing
    MsgBox "Collected totals are " & ActiveSheet.UsedRange.Rows.Count

End Sub

I tried to make it error-free.

Except this is the Web, and whether a server located somewhere on the globe will respond with something you expect, is completely out of your control.

This instruction:

.Open "GET", mlink & y, False

Succeeds under normal circumstances, but one day the site will be down for maintenance, or whatever - and you'll get a run-time error, be it here or at the .Send call.

Code that doesn't handle errors is code that is written for the "happy path" - it's code that works well, until it doesn't. And then, when one thing goes wrong, everything burst up in flames, in a quite inelegant way. There are ways to graciously handle run-time errors.

Use them.

On Error GoTo CleanFail

Looks like you like scraping stuff. That's great, but at one point you need to solve the more generic problem, and move the URL from a local Const to a parameter.

Consider implementing the website-specifics as interface implementations.

@Interface
Option Explicit

Public Function Parse(ByVal url As String) As VBA.Collection
End Function

And when you start implementing interfaces in VBA, you'll find that VBA will refuse to compile when you have an implemented public interface member with an underscore in its name - so you might as well drop that habit now.

The indentation is wrong. Get the latest Rubberduck and use its Smart Indenter. Rubberduck will also warn you about other things, e.g. multiple declarations in a single instruction, single-letter identifiers, and As New, which literally makes an indestructible object - which is usually a very bad idea. Try adding this instruction:

Set http = Nothing
Debug.Print http Is Nothing

If you expected that to print True, you've fallen prey to the As New "feature". Best stay away from that when you want to control your objects' life time.


Shortening code to create a JFrame I'm new to Java and have just started making JFrames. I wrote a program for a game project I'm working on and would like help on where I can improve. I'm not new to programming, I also know HTML and Python well.

I believe one of the goals for all programmers is to make a useful/fun program as simple and short as possible.

I would like pointers/ideas/improvements for my Java program on how I can make it shorter, where I should aim for and how I can make it simpler.

package main;

import java.awt.Dimension;
import java.awt.Toolkit;

import javax.swing.ImageIcon;
import javax.swing.JFrame;

public class Game {

    //Set Variables to make it easier for myself
    public static double VER = 1.0;
    public static String STATE = "ALPHA";
    public static int WIDTH = 400;
    public static int HEIGHT = 400;
    public static int SCALE = 1;

    public static void main(String [ ] args) {
        //Create JFrame
        JFrame launcher = new JFrame("Infinite Doom " + STATE + " " + VER);
        launcher.pack();

        //Settings for JFrame
        launcher.setVisible(true);
        launcher.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        launcher.setResizable(false);
        launcher.setIconImage(new ImageIcon("Logo.png").getImage());
        launcher.setSize(WIDTH, HEIGHT);

        //Check Screen Res and centre window
        Dimension dim = Toolkit.getDefaultToolkit().getScreenSize();
        launcher.setLocation(dim.width/2-launcher.getSize().width/2, dim.height/2-launcher.getSize().height/2);
         }
    }

Just about the only thing I could comment on is your indentation the last two brackets

        launcher.setLocation(dim.width/2-launcher.getSize().width/2, dim.height/2-launcher.getSize().height/2);
         }
    }

should be:

        launcher.setLocation(dim.width/2-launcher.getSize().width/2, dim.height/2launcher.getSize().height/2);
     }
}

Your variables:
VER, STATE, WIDTH, HEIGHT and SCALE could be set to final.

And last but maybe irrelevant. SCALE is not used, but maybe you're going to use it later. What do it know. :)


Stack without use of STL - Version 1.1 I had already posted this question at here
and have tried to incorporate some of the suggestions given. Kindly let me know if there are still some concerns/suggestions/improvements in the code mentioned below.

#include"iostream"
#define MAX_SIZE 5
class Mystack
{
private:
    int A[MAX_SIZE];
    int top;
public:
    Mystack();
    void push(int x);
    int pop();
    int topElement();
    bool isEmpty();
    bool isFull();
    void print();
};
    Mystack::Mystack()
    {
        top = -1;
    }
    void Mystack::push(int x)
    {
        if(isFull())
        {
            std::cout << "Stack Overflow" << std::endl;
            return;
        }
        else
        {
            top++;
            A[top] = x;
        }
    }
    bool Mystack::isFull()
    {
        if (top == MAX_SIZE - 1)
        {
            std::cout << "Stack is full"<< std::endl;
            return true;
        }
        else
        {
            std::cout << "Stack is not full"<<std::endl;
            return false;
        }

    }
    int Mystack::pop()
    {
        if (isEmpty())
        {
            std::cout << "Stack Underflow" << std::endl;
            return 0;
        }
        else
        {
            std::cout << "The popped element is" << A[top];
            return A[top--];
        }
    }
    bool Mystack::isEmpty()
    {
        if (top == -1)
        {
            std::cout << "Stack is empty" << std::endl;
            return true;
        }
        else
        {
            std::cout << "Stack is 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;
    int num,choice = 1;
    while (choice >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 << "7. IsFull" << std::endl;
        std::cout << "Enter the choice" << std::endl;
        std::cin >> choice;
        switch (choice)
        {
        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;
        case 7:
            s1.isFull();
            break;
        }
    }
    system("pause");
}

Don't mix a "library" class and input/output that is a bad style. If you really need to output some debug output, use a logger.
In push(), in case the a stack is full, you need to throw an exception or return an error code. Don't just silently ignore the error.
Same problem with the pop() method. Don't just ignore the error, notify the caller. And do not simply return 0, because it is a valid value that can be stored in a stack, so caller will not be able to distinguish error from successful call.
isFull() can be rewritten as a one-liner:

isFull() { return top == MAX_SIZE - 1; }

Don't use #define to define a constant. Use const int. #define works as a simple text substitution and is a very old and very outdated way to define constants.
Add templates to your stack to support not only a stack of ints, but a stack of any type.
Is it a feature that your stack has a limited size? If yes, consider specifying max size through the constructor. If not, extend your internal array when the limit is reached.
The top variable is pointing to the last set element. Why not to change invariant to "point to the first free element" and replace this awkward top = -1 initialization with top = 0?
print() should not be in your stack class. Provide a generic way to iterate though your stack (read about how STL iterators are implemented or implement your own iterator) and let a user decide what to do with each value from a stack while iterating it.
You can replace top++; A[top] = x; with A[++top] = x. You are using similar style in other place of your class anyway.
Consider renaming array A. The name is not descriptive at all.
Well... "Mystack" name is awkward, rename it to "Stack" :)